Commit 99965378 authored by Ben Hutchings's avatar Ben Hutchings Committed by J. Bruce Fields

nfsd: check permissions when setting ACLs

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: default avatarDavid 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: default avatarJ. Bruce Fields <bfields@redhat.com>
parent 485e71e8
...@@ -104,22 +104,21 @@ static __be32 nfsacld_proc_setacl(struct svc_rqst * rqstp, ...@@ -104,22 +104,21 @@ static __be32 nfsacld_proc_setacl(struct svc_rqst * rqstp,
goto out; goto out;
inode = d_inode(fh->fh_dentry); 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); error = fh_want_write(fh);
if (error) if (error)
goto out_errno; 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) if (error)
goto out_drop_write; goto out_drop_lock;
error = inode->i_op->set_acl(inode, argp->acl_default, error = set_posix_acl(inode, ACL_TYPE_DEFAULT, argp->acl_default);
ACL_TYPE_DEFAULT);
if (error) if (error)
goto out_drop_write; goto out_drop_lock;
fh_unlock(fh);
fh_drop_write(fh); fh_drop_write(fh);
...@@ -131,7 +130,8 @@ static __be32 nfsacld_proc_setacl(struct svc_rqst * rqstp, ...@@ -131,7 +130,8 @@ static __be32 nfsacld_proc_setacl(struct svc_rqst * rqstp,
posix_acl_release(argp->acl_access); posix_acl_release(argp->acl_access);
posix_acl_release(argp->acl_default); posix_acl_release(argp->acl_default);
return nfserr; return nfserr;
out_drop_write: out_drop_lock:
fh_unlock(fh);
fh_drop_write(fh); fh_drop_write(fh);
out_errno: out_errno:
nfserr = nfserrno(error); nfserr = nfserrno(error);
......
...@@ -95,22 +95,20 @@ static __be32 nfsd3_proc_setacl(struct svc_rqst * rqstp, ...@@ -95,22 +95,20 @@ static __be32 nfsd3_proc_setacl(struct svc_rqst * rqstp,
goto out; goto out;
inode = d_inode(fh->fh_dentry); 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); error = fh_want_write(fh);
if (error) if (error)
goto out_errno; 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) if (error)
goto out_drop_write; goto out_drop_lock;
error = inode->i_op->set_acl(inode, argp->acl_default, error = set_posix_acl(inode, ACL_TYPE_DEFAULT, argp->acl_default);
ACL_TYPE_DEFAULT);
out_drop_write: out_drop_lock:
fh_unlock(fh);
fh_drop_write(fh); fh_drop_write(fh);
out_errno: out_errno:
nfserr = nfserrno(error); nfserr = nfserrno(error);
......
...@@ -770,9 +770,6 @@ nfsd4_set_nfs4_acl(struct svc_rqst *rqstp, struct svc_fh *fhp, ...@@ -770,9 +770,6 @@ nfsd4_set_nfs4_acl(struct svc_rqst *rqstp, struct svc_fh *fhp,
dentry = fhp->fh_dentry; dentry = fhp->fh_dentry;
inode = d_inode(dentry); inode = d_inode(dentry);
if (!inode->i_op->set_acl || !IS_POSIXACL(inode))
return nfserr_attrnotsupp;
if (S_ISDIR(inode->i_mode)) if (S_ISDIR(inode->i_mode))
flags = NFS4_ACL_DIR; flags = NFS4_ACL_DIR;
...@@ -782,16 +779,19 @@ nfsd4_set_nfs4_acl(struct svc_rqst *rqstp, struct svc_fh *fhp, ...@@ -782,16 +779,19 @@ nfsd4_set_nfs4_acl(struct svc_rqst *rqstp, struct svc_fh *fhp,
if (host_error < 0) if (host_error < 0)
goto out_nfserr; 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) if (host_error < 0)
goto out_release; goto out_drop_lock;
if (S_ISDIR(inode->i_mode)) { if (S_ISDIR(inode->i_mode)) {
host_error = inode->i_op->set_acl(inode, dpacl, host_error = set_posix_acl(inode, ACL_TYPE_DEFAULT, dpacl);
ACL_TYPE_DEFAULT);
} }
out_release: out_drop_lock:
fh_unlock(fhp);
posix_acl_release(pacl); posix_acl_release(pacl);
posix_acl_release(dpacl); posix_acl_release(dpacl);
out_nfserr: out_nfserr:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment