On Thu, Jul 10, 2014 at 12:26 AM, Frank Filz <ffilz...@mindspring.com> wrote: >> On Wed, Jul 9, 2014 at 7:06 PM, Trond Myklebust >> <trond.mykleb...@primarydata.com> wrote: >> > On Wed, Jul 9, 2014 at 6:42 PM, Frank Filz <ffilz...@mindspring.com> >> wrote: >> >>> On Wed, Jul 9, 2014 at 5:54 PM, Frank S. Filz >> >>> <ffilz...@mindspring.com> >> >>> wrote: >> >>> > From: "Frank S. Filz" <ffilz...@mindspring.com> >> >>> > >> >>> > The NFS v4 client sends a COMPOUND with an OPEN and an ACCESS. >> >>> > >> >>> > The ACCESS is required to verify an open for read is actually >> >>> > allowed because RFC 3530 indicates OPEN for read only must succeed >> >>> > for an execute only file. >> >>> > >> >>> > The old code expected to have read access if the requested access >> >>> > was O_RDWR. >> >>> > >> >>> > We can expect the OPEN to properly permission check as long as the >> >>> > open is O_WRONLY or O_RDWR. >> >>> > >> >>> > Signed-off-by: Frank S. Filz <ffilz...@mindspring.com> >> >>> > --- >> >>> > fs/nfs/nfs4proc.c | 25 ++++++++++++++++++++----- >> >>> > 1 file changed, 20 insertions(+), 5 deletions(-) >> >>> > >> >>> > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index >> >>> > 4bf3d97..9742054 100644 >> >>> > --- a/fs/nfs/nfs4proc.c >> >>> > +++ b/fs/nfs/nfs4proc.c >> >>> > @@ -1966,15 +1966,30 @@ static int nfs4_opendata_access(struct >> >>> rpc_cred *cred, >> >>> > return 0; >> >>> > >> >>> > mask = 0; >> >>> > - /* don't check MAY_WRITE - a newly created file may not have >> >>> > - * write mode bits, but POSIX allows the creating process to >> >>> > write. >> >>> > - * use openflags to check for exec, because fmode won't >> >>> > - * always have FMODE_EXEC set when file open for exec. */ >> >>> > + /* Don't trust the permission check on OPEN if open for exec or >> for >> >>> > + * read only. Since FMODE_EXEC doesn't go across the wire, the >> server >> >>> > + * has no way to distinguish between an open to read an >> >>> > + executable >> >>> file >> >>> > + * and an open to read a readable file. Write access is >> >>> > properly >> checked >> >>> > + * and permission SHOULD always be granted if the file was >> >>> > + created as >> >>> a >> >>> > + * result of this OPEN, no matter what mode the file was >> >>> > created >> with. >> >>> > + * >> >>> > + * NOTE: If the case of a OPEN CREATE READ-ONLY with a >> >>> > + mode that >> >>> does >> >>> > + * not allow read access, this test will produce an >> >>> > incorrect >> >>> > + * EACCES error. >> >>> > + */ >> >>> > if (openflags & __FMODE_EXEC) { >> >>> > /* ONLY check for exec rights */ >> >>> > mask = MAY_EXEC; >> >>> > - } else if (fmode & FMODE_READ) >> >>> > + } else if (!(fmode & FMODE_WRITE)) { >> >>> > + /* In case the file was execute only, check for read >> >>> > permission >> >>> > + * ONLY if write access was not requested. It is >> >>> > expected that >> >>> > + * an OPEN for write will fail if the file is execute >> >>> > only. >> >>> > + * Note that if the file was newly created, the fmode >> >>> > SHOULD >> >>> > + * include FMODE_WRITE, especially if the file will be >> >>> > created >> >>> > + * with a restrictive mode. >> >>> > + */ >> >>> > mask = MAY_READ; >> >>> > + } >> >>> >> >>> This looks wrong. AFAICS it will allow you to open an existing file >> >>> which has - wx permissions (i.e. no read permissions) for O_RDWR. >> >>> That should not be permitted under POSIX rules. >> >> >> >> The server permission checks the OPEN, this only affects the subsequent >> ACCESS. >> >> >> >> The server will fail the OPEN with NFS4_ERR_ACCESS if the open is for >> read/write and the file has write-execute permission. >> > >> > RFC3530bis draft 33 (6.2.1.3.1. Discussion of Mask Attributes) states >> > that for both the OPEN and the READ operations, "Servers SHOULD allow >> > a user the ability to read the data of the file when only the >> > ACE4_EXECUTE access mask bit is allowed". RFC5561 has the same >> > language. >> >> Oops. Sorry, the correct sub-sub-sub-sub-....paragraph is this one: >> >> Permission to execute a file. >> >> Servers SHOULD allow a user the ability to read the data of the >> file when only the ACE4_EXECUTE access mask bit is allowed. >> This is because there is no way to execute a file without >> reading the contents. Though a server may treat ACE4_EXECUTE >> and ACE4_READ_DATA bits identically when deciding to permit a >> READ operation, it SHOULD still allow the two bits to be set >> independently in ACLs, and MUST distinguish between them when >> replying to ACCESS operations. In particular, servers SHOULD >> NOT silently turn on one of the two bits when the other is set, >> as that would make it impossible for the client to correctly >> enforce the distinction between read and execute permissions. >> >> >> > To me that translates as saying that the server SHOULD accept an >> > OPEN(SHARE_ACCESS_READ|SHARE_ACCESS_WRITE) request in the above >> > situation. >> >> Same conclusion, though.... > > When I read that paragraph, my interpretation is that OPEN (and READ) should > be permission checked normally, however, if ONLY execute permission is > granted, and the OPEN is read only (and READ of course is read only) then > permission would be granted for the purpose of execution. But if any other > combination of bits was allowed, then the paragraph doesn't apply. Thus an > OPEN SHARE_ACCESS_READ | SHARE_ACCESS_WRITE must fail since write access was > not granted (if it was, the exception doesn't apply to my reading). >
Where does that paragraph say anything about SHARE_WRITE, or even mention the word "only"? All it says is that as far as the OPEN and READ operations are concerned, ACE4_EXECUTE == ACE4_READ_DATA, whereas for the ACCESS operation, they differ. -- Trond Myklebust Linux NFS client maintainer, PrimaryData trond.mykleb...@primarydata.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/