Hi,
On Mon, Aug 14, 2017, Michael Paquier <michael.paqu...@gmail.com> wrote:
>Attached is a set of 3 patches:
I tried to review the patch and firstly patch applies cleanly without any
noise.
>- 0001 removes ALLOW_DANGEROUS_LO_FUNCTIONS
I think it is better to remove "ALLOW_DANGEROUS_LO_FUNCTIONS" from
pg_config_manual.h as well.
>- 0002 replaces the superuser checks with GRANT permissions
>- 0003 refactors the LO code to improve the ACL handling. Note that
>this patch introduces a more debatable change: now there is no
>distinction between INV_WRITE and INV_WRITE|INV_READ, as when
>INV_WRITE is used INV_READ is implied. The patch as proposed does a
>complete split of both permissions to give a system more natural and
>more flexible. The current behavior is documented.
>@@ -163,22 +150,16 @@ lo_read(int fd, char *buf, int len)
> ....
> + if ((lobj->flags & IFS_RDLOCK) == 0)
>+ ereport(ERROR,
>+ (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
>+ errmsg("large object descriptor %d was not opened for reading",
>+ fd)));
Do we ever reach this error? Because per my understanding
1) if the application didn't call lo_open before lo_read, then file
descriptor validation in the beginning of this function should capture it.
2) if the application called lo_open only with write mode should be able to
read as well per documentation.
Ok, in the inv_open(), IFS_RDLOCK is set only when explicit READ mode is
requested. So, it causes lo_read failure when large-object is opened in
WRITE mode? I think documented behavior is more appropriate and post ACL
checks for select and update permissions in inv_open(), IFS_RDLOCK flag
should be set regardless of mode specified.
Thanks & Regards,
Vaishnavi,
Fujitsu Australia.
