nfsd: check permissions when setting ACLs
commit 999653786df6954a31044528ac3f7a5dadca08f4 upstream. 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 commit4ac7249e
, 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> Signed-off-by: J. Bruce Fields <bfields@redhat.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
parent
c3fa141c1f
commit
412cfeec2f
3 changed files with 26 additions and 28 deletions
|
@ -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 @@ out:
|
||||||
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,
|
||||||
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);
|
||||||
if (error)
|
|
||||||
goto out_drop_write;
|
|
||||||
error = inode->i_op->set_acl(inode, argp->acl_default,
|
|
||||||
ACL_TYPE_DEFAULT);
|
|
||||||
|
|
||||||
out_drop_write:
|
error = set_posix_acl(inode, ACL_TYPE_ACCESS, argp->acl_access);
|
||||||
|
if (error)
|
||||||
|
goto out_drop_lock;
|
||||||
|
error = set_posix_acl(inode, ACL_TYPE_DEFAULT, argp->acl_default);
|
||||||
|
|
||||||
|
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,
|
||||||
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,
|
||||||
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:
|
||||||
|
|
Loading…
Add table
Reference in a new issue