2012-10-02 21:49:56

by Weston Andros Adamson

[permalink] [raw]
Subject: [PATCH 0/2] NFSv4: fix corner cases with ACCESS in OPEN

My recent "ACCESS in OPEN" patch that fixed permissions due to OPEN operations
having no way to differentiate between READ and EXEC had several issues:

- when files are opened O_EXCL, ACCESS will always return permission denied
for all bits until close is called.

- POSIX allows processes to write to files without write permissions if
that process created the file



2012-10-02 21:49:56

by Weston Andros Adamson

[permalink] [raw]
Subject: [PATCH 1/2] NFSv4: don't check MAY_WRITE access bit in OPEN

Don't check MAY_WRITE as a newly created file may not have write mode bits,
but POSIX allows the creating process to write regardless.
This is ok because NFSv4 OPEN ops handle write permissions correctly -
the ACCESS in the OPEN compound is to differentiate READ v EXEC permissions.

Signed-off-by: Weston Andros Adamson <[email protected]>
---
fs/nfs/nfs4proc.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 1e0faf9..ccada68 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -1661,10 +1661,10 @@ 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 */
if (fmode & FMODE_READ)
mask |= MAY_READ;
- if (fmode & FMODE_WRITE)
- mask |= MAY_WRITE;
if (fmode & FMODE_EXEC)
mask |= MAY_EXEC;

@@ -1673,7 +1673,7 @@ static int nfs4_opendata_access(struct rpc_cred *cred,
nfs_access_set_mask(&cache, opendata->o_res.access_result);
nfs_access_add_cache(state->inode, &cache);

- if ((mask & ~cache.mask & (MAY_READ | MAY_WRITE | MAY_EXEC)) == 0)
+ if ((mask & ~cache.mask & (MAY_READ | MAY_EXEC)) == 0)
return 0;

/* even though OPEN succeeded, access is denied. Close the file */
--
1.7.9.6 (Apple Git-31.1)


2012-10-02 21:49:57

by Weston Andros Adamson

[permalink] [raw]
Subject: [PATCH 2/2] NFSv4: don't put ACCESS in OPEN compound if O_EXCL

Don't put an ACCESS op in OPEN compound if O_EXCL, because ACCESS
will return permission denied for all bits until close.

Signed-off-by: Weston Andros Adamson <[email protected]>
---
fs/nfs/nfs4proc.c | 12 +++++++++---
fs/nfs/nfs4xdr.c | 12 ++++++++----
include/linux/nfs_xdr.h | 1 +
3 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index ccada68..21cfac7 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -862,9 +862,15 @@ static struct nfs4_opendata *nfs4_opendata_alloc(struct dentry *dentry,
p->o_arg.fh = NFS_FH(dir);
p->o_arg.open_flags = flags;
p->o_arg.fmode = fmode & (FMODE_READ|FMODE_WRITE);
- /* ask server to check for all possible rights as results are cached */
- p->o_arg.access = NFS4_ACCESS_READ | NFS4_ACCESS_MODIFY |
- NFS4_ACCESS_EXTEND | NFS4_ACCESS_EXECUTE;
+ /* don't put an ACCESS op in OPEN compound if O_EXCL, because ACCESS
+ * will return permission denied for all bits until close */
+ if (!(flags & O_EXCL)) {
+ /* ask server to check for all possible rights as results
+ * are cached */
+ p->o_arg.access = NFS4_ACCESS_READ | NFS4_ACCESS_MODIFY |
+ NFS4_ACCESS_EXTEND | NFS4_ACCESS_EXECUTE;
+ p->o_res.access_request = p->o_arg.access;
+ }
p->o_arg.clientid = server->nfs_client->cl_clientid;
p->o_arg.id.create_time = ktime_to_ns(sp->so_seqid.create_time);
p->o_arg.id.uniquifier = sp->so_seqid.owner_id;
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 657483c..0d60305 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -2224,7 +2224,8 @@ static void nfs4_xdr_enc_open(struct rpc_rqst *req, struct xdr_stream *xdr,
encode_putfh(xdr, args->fh, &hdr);
encode_open(xdr, args, &hdr);
encode_getfh(xdr, &hdr);
- encode_access(xdr, args->access, &hdr);
+ if (args->access)
+ encode_access(xdr, args->access, &hdr);
encode_getfattr_open(xdr, args->bitmask, args->open_bitmap, &hdr);
encode_nops(&hdr);
}
@@ -2261,7 +2262,8 @@ static void nfs4_xdr_enc_open_noattr(struct rpc_rqst *req,
encode_sequence(xdr, &args->seq_args, &hdr);
encode_putfh(xdr, args->fh, &hdr);
encode_open(xdr, args, &hdr);
- encode_access(xdr, args->access, &hdr);
+ if (args->access)
+ encode_access(xdr, args->access, &hdr);
encode_getfattr(xdr, args->bitmask, &hdr);
encode_nops(&hdr);
}
@@ -6239,7 +6241,8 @@ static int nfs4_xdr_dec_open(struct rpc_rqst *rqstp, struct xdr_stream *xdr,
status = decode_getfh(xdr, &res->fh);
if (status)
goto out;
- decode_access(xdr, &res->access_supported, &res->access_result);
+ if (res->access_request)
+ decode_access(xdr, &res->access_supported, &res->access_result);
decode_getfattr(xdr, res->f_attr, res->server);
out:
return status;
@@ -6288,7 +6291,8 @@ static int nfs4_xdr_dec_open_noattr(struct rpc_rqst *rqstp,
status = decode_open(xdr, res);
if (status)
goto out;
- decode_access(xdr, &res->access_supported, &res->access_result);
+ if (res->access_request)
+ decode_access(xdr, &res->access_supported, &res->access_result);
decode_getfattr(xdr, res->f_attr, res->server);
out:
return status;
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 655490d..a73ea89 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -369,6 +369,7 @@ struct nfs_openres {
struct nfs4_string *owner;
struct nfs4_string *group_owner;
struct nfs4_sequence_res seq_res;
+ __u32 access_request;
__u32 access_supported;
__u32 access_result;
};
--
1.7.9.6 (Apple Git-31.1)