nfsd: fix incorrect umasks

commit 880a3a5325489a143269a8e172e7563ebf9897bc upstream.

We're neglecting to clear the umask after it's set, which can cause a
later unrelated rpc to (incorrectly) use the same umask if it happens to
be processed by the same thread.

There's a more subtle problem here too:

An NFSv4 compound request is decoded all in one pass before any
operations are executed.

Currently we're setting current->fs->umask at the time we decode the
compound.  In theory a single compound could contain multiple creates
each setting a umask.  In that case we'd end up using whichever umask
was passed in the *last* operation as the umask for all the creates,
whether that was correct or not.

So, we should just be saving the umask at decode time and waiting to set
it until we actually process the corresponding operation.

In practice it's unlikely any client would do multiple creates in a
single compound.  And even if it did they'd likely be from the same
process (hence carry the same umask).  So this is a little academic, but
we should get it right anyway.

Fixes: 47057abde515 (nfsd: add support for the umask attribute)
Cc: stable@vger.kernel.org
Reported-by: Lucash Stach <l.stach@pengutronix.de>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 8ce60986fb750bc9dad37122208ba65b6c8f2caf..5b6ff168d11aaeb0e9046329a96ced18621f8672 100644 (file)
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -32,6 +32,7 @@
  *  NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
  *  SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
+#include <linux/fs_struct.h>
 #include <linux/file.h>
 #include <linux/falloc.h>
 #include <linux/slab.h>
@@ -252,11 +253,13 @@ do_open_lookup(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, stru
                 * Note: create modes (UNCHECKED,GUARDED...) are the same
                 * in NFSv4 as in v3 except EXCLUSIVE4_1.
                 */
+               current->fs->umask = open->op_umask;
                status = do_nfsd_create(rqstp, current_fh, open->op_fname.data,
                                        open->op_fname.len, &open->op_iattr,
                                        *resfh, open->op_createmode,
                                        (u32 *)open->op_verf.data,
                                        &open->op_truncate, &open->op_created);
+               current->fs->umask = 0;
 
                if (!status && open->op_label.len)
                        nfsd4_security_inode_setsecctx(*resfh, &open->op_label, open->op_bmval);
@@ -608,6 +611,7 @@ nfsd4_create(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
        if (status)
                return status;
 
+       current->fs->umask = create->cr_umask;
        switch (create->cr_type) {
        case NF4LNK:
                status = nfsd_symlink(rqstp, &cstate->current_fh,
@@ -616,20 +620,22 @@ nfsd4_create(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
                break;
 
        case NF4BLK:
+               status = nfserr_inval;
                rdev = MKDEV(create->cr_specdata1, create->cr_specdata2);
                if (MAJOR(rdev) != create->cr_specdata1 ||
                    MINOR(rdev) != create->cr_specdata2)
-                       return nfserr_inval;
+                       goto out_umask;
                status = nfsd_create(rqstp, &cstate->current_fh,
                                     create->cr_name, create->cr_namelen,
                                     &create->cr_iattr, S_IFBLK, rdev, &resfh);
                break;
 
        case NF4CHR:
+               status = nfserr_inval;
                rdev = MKDEV(create->cr_specdata1, create->cr_specdata2);
                if (MAJOR(rdev) != create->cr_specdata1 ||
                    MINOR(rdev) != create->cr_specdata2)
-                       return nfserr_inval;
+                       goto out_umask;
                status = nfsd_create(rqstp, &cstate->current_fh,
                                     create->cr_name, create->cr_namelen,
                                     &create->cr_iattr,S_IFCHR, rdev, &resfh);
@@ -673,6 +679,8 @@ nfsd4_create(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
        fh_dup2(&cstate->current_fh, &resfh);
 out:
        fh_put(&resfh);
+out_umask:
+       current->fs->umask = 0;
        return status;
 }
 

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 2c61c6b8ae0976209cbb8a443fb5db1a50d5fdad..df2b8849a63bfb404dcef3feae8bb851615a5f93 100644 (file)
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -33,7 +33,6 @@
  *  SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-#include <linux/fs_struct.h>
 #include <linux/file.h>
 #include <linux/slab.h>
 #include <linux/namei.h>
@@ -683,7 +682,7 @@ nfsd4_decode_create(struct nfsd4_compoundargs *argp, struct nfsd4_create *create
 
        status = nfsd4_decode_fattr(argp, create->cr_bmval, &create->cr_iattr,
                                    &create->cr_acl, &create->cr_label,
-                                   &current->fs->umask);
+                                   &create->cr_umask);
        if (status)
                goto out;
 
@@ -928,7 +927,6 @@ nfsd4_decode_open(struct nfsd4_compoundargs *argp, struct nfsd4_open *open)
        case NFS4_OPEN_NOCREATE:
                break;
        case NFS4_OPEN_CREATE:
-               current->fs->umask = 0;
                READ_BUF(4);
                open->op_createmode = be32_to_cpup(p++);
                switch (open->op_createmode) {
@@ -936,7 +934,7 @@ nfsd4_decode_open(struct nfsd4_compoundargs *argp, struct nfsd4_open *open)
                case NFS4_CREATE_GUARDED:
                        status = nfsd4_decode_fattr(argp, open->op_bmval,
                                &open->op_iattr, &open->op_acl, &open->op_label,
-                               &current->fs->umask);
+                               &open->op_umask);
                        if (status)
                                goto out;
                        break;
@@ -951,7 +949,7 @@ nfsd4_decode_open(struct nfsd4_compoundargs *argp, struct nfsd4_open *open)
                        COPYMEM(open->op_verf.data, NFS4_VERIFIER_SIZE);
                        status = nfsd4_decode_fattr(argp, open->op_bmval,
                                &open->op_iattr, &open->op_acl, &open->op_label,
-                               &current->fs->umask);
+                               &open->op_umask);
                        if (status)
                                goto out;
                        break;

diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index 1e4edbf70052bf5f6c8c0af06e0d7bf1812e80a5..aa4375eac47549746a561b9eddf47e0df7ff7879 100644 (file)
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -118,6 +118,7 @@ struct nfsd4_create {
        } u;
        u32             cr_bmval[3];        /* request */
        struct iattr    cr_iattr;           /* request */
+       int             cr_umask;           /* request */
        struct nfsd4_change_info  cr_cinfo; /* response */
        struct nfs4_acl *cr_acl;
        struct xdr_netobj cr_label;
@@ -228,6 +229,7 @@ struct nfsd4_open {
        u32             op_why_no_deleg;    /* response - DELEG_NONE_EXT only */
        u32             op_create;          /* request */
        u32             op_createmode;      /* request */
+       int             op_umask;           /* request */
        u32             op_bmval[3];        /* request */
        struct iattr    op_iattr;           /* UNCHECKED4, GUARDED4, EXCLUSIVE4_1 */
        nfs4_verifier   op_verf __attribute__((aligned(32)));