Re: [Qemu-devel] [for-2.10 PATCH] 9pfs: local: fix fchmodat_nofollow() limitations

[Greg Kurz]

On Tue, 8 Aug 2017 15:28:35 -0500
Eric Blake <address@hidden> wrote:

> On 08/08/2017 03:24 PM, Eric Blake wrote:
> > On 08/08/2017 03:10 PM, Philippe Mathieu-Daudé wrote:  
> >>> Technically, POSIX says (and 'man 2 open' agrees, modulo the fact that
> >>> Linux still lacks O_SEARCH) that you MUST provide one of the 5 access
> >>> modes (they are O_RDONLY, O_RDWR, O_WRONLY, O_EXEC, and O_SEARCH;
> >>> although POSIX allows O_EXEC and O_SEARCH to be the same bit pattern),
> >>> and then bitwise-or any additional flags.  So the usage here is correct.
> >>>  
> >   
> >> Oh ok. I didn't think of that, just checked Linux manpage:
> >>
> >>    O_PATH (since Linux 2.6.39)
> >>
> >>           When O_PATH is specified in flags, flag bits other than
> >>           O_CLOEXEC, O_DIRECTORY, and O_NOFOLLOW are ignored.  
> > 
> > There are access modes (5 in POSIX), and then flag bits (O_NONBLOCK
> > being one of the flag bits, and therefore ignored when O_PATH is true).
> > Presumably, the author was being careful by mentioning "flag bits" (and
> > thereby implicitly meaning that O_RDONLY is NOT ignored when using
> > O_PATH).  But I'm not _quite_ sure whether O_PATH should be considered a
> > sixth access mode, or a flag bit, and the Linux man page doesn't help on
> > that front ;)  Hmm - if you treat O_PATH as an access mode rather than a
> > flag bit, then O_RDONLY | O_PATH no longer makes sense at all (you can't
> > mix two modes at once).  Maybe we should file a bug report against the
> > man page to get clarification.  
> 
> Quoting my version of 'man 2 open'
> 
>        The  argument  flags  must  include  one of the following access
> modes:
>        O_RDONLY, O_WRONLY, or O_RDWR.  These request opening  the  file
> read-
>        only, write-only, or read/write, respectively.
> 
>        In addition, zero or more file creation flags and file status
> flags can
>        be bitwise-or'd in flags.   The  file  creation  flags  are
> O_CLOEXEC,
>        O_CREAT,  O_DIRECTORY,  O_EXCL,  O_NOCTTY,  O_NOFOLLOW,
> O_TMPFILE, and
>        O_TRUNC.  The file status flags are all of the remaining  flags
> listed
>        below.   The  distinction between these two groups of flags is
> that the
>        file status flags can be retrieved and (in some  cases)
> modified;  see
>        fcntl(2) for details.
> 
> and fcntl() lets you see whether an fd was opened with O_PATH, which
> makes it a file status flag.  Well, except that fcntl() also lets you
> see which mode an fd was opened with (such as O_WRONLY).  Hmm - still fuzzy.
> 

I agree the documentation is unclear, but in linux/fs/open.c we have:

static inline int build_open_flags(int flags, umode_t mode, struct open_flags 
*op)
{
        int lookup_flags = 0;
        int acc_mode = ACC_MODE(flags);
[...]
        } else if (flags & O_PATH) {
                /*
                 * If we have O_PATH in the open flag. Then we
                 * cannot have anything other than the below set of flags
                 */
                flags &= O_DIRECTORY | O_NOFOLLOW | O_PATH;
                acc_mode = 0;
        }

This seems to indicate that the access mode is simply ignored.

So I guess it is ok to drop O_RDONLY, even if it isn't clearly documented
in the manpage... I don't have a strong opinion anyway.

pgpD8VeAfK9Nu.pgp
Description: OpenPGP digital signature