Skip to content

Commit 9996537

Browse files
bwhacksJ. Bruce Fields
authored and
J. Bruce Fields
committed
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 4ac7249, 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: 4ac7249 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>
1 parent 485e71e commit 9996537

File tree

3 files changed

+25
-27
lines changed

3 files changed

+25
-27
lines changed

Diff for: fs/nfsd/nfs2acl.c

+10-10
Original file line numberDiff line numberDiff line change
@@ -104,22 +104,21 @@ static __be32 nfsacld_proc_setacl(struct svc_rqst * rqstp,
104104
goto out;
105105

106106
inode = d_inode(fh->fh_dentry);
107-
if (!IS_POSIXACL(inode) || !inode->i_op->set_acl) {
108-
error = -EOPNOTSUPP;
109-
goto out_errno;
110-
}
111107

112108
error = fh_want_write(fh);
113109
if (error)
114110
goto out_errno;
115111

116-
error = inode->i_op->set_acl(inode, argp->acl_access, ACL_TYPE_ACCESS);
112+
fh_lock(fh);
113+
114+
error = set_posix_acl(inode, ACL_TYPE_ACCESS, argp->acl_access);
117115
if (error)
118-
goto out_drop_write;
119-
error = inode->i_op->set_acl(inode, argp->acl_default,
120-
ACL_TYPE_DEFAULT);
116+
goto out_drop_lock;
117+
error = set_posix_acl(inode, ACL_TYPE_DEFAULT, argp->acl_default);
121118
if (error)
122-
goto out_drop_write;
119+
goto out_drop_lock;
120+
121+
fh_unlock(fh);
123122

124123
fh_drop_write(fh);
125124

@@ -131,7 +130,8 @@ static __be32 nfsacld_proc_setacl(struct svc_rqst * rqstp,
131130
posix_acl_release(argp->acl_access);
132131
posix_acl_release(argp->acl_default);
133132
return nfserr;
134-
out_drop_write:
133+
out_drop_lock:
134+
fh_unlock(fh);
135135
fh_drop_write(fh);
136136
out_errno:
137137
nfserr = nfserrno(error);

Diff for: fs/nfsd/nfs3acl.c

+7-9
Original file line numberDiff line numberDiff line change
@@ -95,22 +95,20 @@ static __be32 nfsd3_proc_setacl(struct svc_rqst * rqstp,
9595
goto out;
9696

9797
inode = d_inode(fh->fh_dentry);
98-
if (!IS_POSIXACL(inode) || !inode->i_op->set_acl) {
99-
error = -EOPNOTSUPP;
100-
goto out_errno;
101-
}
10298

10399
error = fh_want_write(fh);
104100
if (error)
105101
goto out_errno;
106102

107-
error = inode->i_op->set_acl(inode, argp->acl_access, ACL_TYPE_ACCESS);
103+
fh_lock(fh);
104+
105+
error = set_posix_acl(inode, ACL_TYPE_ACCESS, argp->acl_access);
108106
if (error)
109-
goto out_drop_write;
110-
error = inode->i_op->set_acl(inode, argp->acl_default,
111-
ACL_TYPE_DEFAULT);
107+
goto out_drop_lock;
108+
error = set_posix_acl(inode, ACL_TYPE_DEFAULT, argp->acl_default);
112109

113-
out_drop_write:
110+
out_drop_lock:
111+
fh_unlock(fh);
114112
fh_drop_write(fh);
115113
out_errno:
116114
nfserr = nfserrno(error);

Diff for: fs/nfsd/nfs4acl.c

+8-8
Original file line numberDiff line numberDiff line change
@@ -770,9 +770,6 @@ nfsd4_set_nfs4_acl(struct svc_rqst *rqstp, struct svc_fh *fhp,
770770
dentry = fhp->fh_dentry;
771771
inode = d_inode(dentry);
772772

773-
if (!inode->i_op->set_acl || !IS_POSIXACL(inode))
774-
return nfserr_attrnotsupp;
775-
776773
if (S_ISDIR(inode->i_mode))
777774
flags = NFS4_ACL_DIR;
778775

@@ -782,16 +779,19 @@ nfsd4_set_nfs4_acl(struct svc_rqst *rqstp, struct svc_fh *fhp,
782779
if (host_error < 0)
783780
goto out_nfserr;
784781

785-
host_error = inode->i_op->set_acl(inode, pacl, ACL_TYPE_ACCESS);
782+
fh_lock(fhp);
783+
784+
host_error = set_posix_acl(inode, ACL_TYPE_ACCESS, pacl);
786785
if (host_error < 0)
787-
goto out_release;
786+
goto out_drop_lock;
788787

789788
if (S_ISDIR(inode->i_mode)) {
790-
host_error = inode->i_op->set_acl(inode, dpacl,
791-
ACL_TYPE_DEFAULT);
789+
host_error = set_posix_acl(inode, ACL_TYPE_DEFAULT, dpacl);
792790
}
793791

794-
out_release:
792+
out_drop_lock:
793+
fh_unlock(fhp);
794+
795795
posix_acl_release(pacl);
796796
posix_acl_release(dpacl);
797797
out_nfserr:

0 commit comments

Comments
 (0)