nfsd: check permissions when setting ACLs
authorBen Hutchings <ben@decadent.org.uk>
Wed, 22 Jun 2016 18:43:35 +0000 (19:43 +0100)
committerJ. Bruce Fields <bfields@redhat.com>
Fri, 24 Jun 2016 16:11:52 +0000 (12:11 -0400)
Use set_posix_acl, which includes proper permission checks, instead of
calling ->set_acl directly.  Without this anyone may be able to grant
themselves permissions to a file by setting the ACL.

Lock the inode to make the new checks atomic with respect to set_acl.
(Also, nfsd was the only caller of set_acl not locking the inode, so I
suspect this may fix other races.)

This also simplifies the code, and ensures our ACLs are checked by
posix_acl_valid.

The permission checks and the inode locking were lost with commit
4ac7249e, which changed nfsd to use the set_acl inode operation directly
instead of going through xattr handlers.

Reported-by: David Sinquin <david@sinquin.eu>
[agreunba@redhat.com: use set_posix_acl]
Fixes: 4ac7249e
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: stable@vger.kernel.org
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
fs/nfsd/nfs2acl.c
fs/nfsd/nfs3acl.c
fs/nfsd/nfs4acl.c

index 1580ea6fd64df15da96b832eeef9f3dc2560e32b..d08cd88155c75278c4607f49c078622bf87ab5ee 100644 (file)
@@ -104,22 +104,21 @@ static __be32 nfsacld_proc_setacl(struct svc_rqst * rqstp,
                goto out;
 
        inode = d_inode(fh->fh_dentry);
-       if (!IS_POSIXACL(inode) || !inode->i_op->set_acl) {
-               error = -EOPNOTSUPP;
-               goto out_errno;
-       }
 
        error = fh_want_write(fh);
        if (error)
                goto out_errno;
 
-       error = inode->i_op->set_acl(inode, argp->acl_access, ACL_TYPE_ACCESS);
+       fh_lock(fh);
+
+       error = set_posix_acl(inode, ACL_TYPE_ACCESS, argp->acl_access);
        if (error)
-               goto out_drop_write;
-       error = inode->i_op->set_acl(inode, argp->acl_default,
-                                    ACL_TYPE_DEFAULT);
+               goto out_drop_lock;
+       error = set_posix_acl(inode, ACL_TYPE_DEFAULT, argp->acl_default);
        if (error)
-               goto out_drop_write;
+               goto out_drop_lock;
+
+       fh_unlock(fh);
 
        fh_drop_write(fh);
 
@@ -131,7 +130,8 @@ out:
        posix_acl_release(argp->acl_access);
        posix_acl_release(argp->acl_default);
        return nfserr;
-out_drop_write:
+out_drop_lock:
+       fh_unlock(fh);
        fh_drop_write(fh);
 out_errno:
        nfserr = nfserrno(error);
index 01df4cd7c753fe12cee4c6d37a5ba9400b6f8f41..0c890347cde3d9559b0b0103c2c2c11825d51fae 100644 (file)
@@ -95,22 +95,20 @@ static __be32 nfsd3_proc_setacl(struct svc_rqst * rqstp,
                goto out;
 
        inode = d_inode(fh->fh_dentry);
-       if (!IS_POSIXACL(inode) || !inode->i_op->set_acl) {
-               error = -EOPNOTSUPP;
-               goto out_errno;
-       }
 
        error = fh_want_write(fh);
        if (error)
                goto out_errno;
 
-       error = inode->i_op->set_acl(inode, argp->acl_access, ACL_TYPE_ACCESS);
+       fh_lock(fh);
+
+       error = set_posix_acl(inode, ACL_TYPE_ACCESS, argp->acl_access);
        if (error)
-               goto out_drop_write;
-       error = inode->i_op->set_acl(inode, argp->acl_default,
-                                    ACL_TYPE_DEFAULT);
+               goto out_drop_lock;
+       error = set_posix_acl(inode, ACL_TYPE_DEFAULT, argp->acl_default);
 
-out_drop_write:
+out_drop_lock:
+       fh_unlock(fh);
        fh_drop_write(fh);
 out_errno:
        nfserr = nfserrno(error);
index 6adabd6049b7199ad72e01e361e3637a88b016ff..71292a0d6f09226e828b327ba139a296dc76a08b 100644 (file)
@@ -770,9 +770,6 @@ nfsd4_set_nfs4_acl(struct svc_rqst *rqstp, struct svc_fh *fhp,
        dentry = fhp->fh_dentry;
        inode = d_inode(dentry);
 
-       if (!inode->i_op->set_acl || !IS_POSIXACL(inode))
-               return nfserr_attrnotsupp;
-
        if (S_ISDIR(inode->i_mode))
                flags = NFS4_ACL_DIR;
 
@@ -782,16 +779,19 @@ nfsd4_set_nfs4_acl(struct svc_rqst *rqstp, struct svc_fh *fhp,
        if (host_error < 0)
                goto out_nfserr;
 
-       host_error = inode->i_op->set_acl(inode, pacl, ACL_TYPE_ACCESS);
+       fh_lock(fhp);
+
+       host_error = set_posix_acl(inode, ACL_TYPE_ACCESS, pacl);
        if (host_error < 0)
-               goto out_release;
+               goto out_drop_lock;
 
        if (S_ISDIR(inode->i_mode)) {
-               host_error = inode->i_op->set_acl(inode, dpacl,
-                                                 ACL_TYPE_DEFAULT);
+               host_error = set_posix_acl(inode, ACL_TYPE_DEFAULT, dpacl);
        }
 
-out_release:
+out_drop_lock:
+       fh_unlock(fhp);
+
        posix_acl_release(pacl);
        posix_acl_release(dpacl);
 out_nfserr: