Patch "nfsd: check permissions when setting ACLs" has been added to the 4.4-stable tree

PATCH [email protected] 1 participant 1 message Jul 11, 2016

Subsystems (2)

Stable branch NFS server

lore

Share & tools

lore

[email protected]

author maintainer Applied

2016-07-11 23:22 UTC #0 lore raw

This is a note to let you know that I've just added the patch titled

nfsd: check permissions when setting ACLs

to the 4.4-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
nfsd-check-permissions-when-setting-acls.patch
and it can be found in the queue-4.4 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 1 line

From 999653786df6954a31044528ac3f7a5dadca08f4 Mon Sep 17 00:00:00 2001

From: Ben Hutchings <[email protected]>
Date: Wed, 22 Jun 2016 19:43:35 +0100
Subject: nfsd: check permissions when setting ACLs

From: Ben Hutchings <[email protected]>

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 commit 4ac7249e, which changed nfsd to use the set_acl inode operation directly instead of going through xattr handlers.

Reported-by David Sinquin <[email protected]>

[[email protected]: use set_posix_acl]

Fixes 4ac7249e

Cc Christoph Hellwig <[email protected]>

Cc Al Viro <[email protected]>

Signed-off-by J. Bruce Fields <[email protected]>

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
 fs/nfsd/nfs2acl.c |   20 ++++++++++----------
 fs/nfsd/nfs3acl.c |   16 +++++++---------
 fs/nfsd/nfs4acl.c |   16 ++++++++--------
 3 files changed, 25 insertions(+), 27 deletions(-)

fs/nfsd/nfs2acl.c +10 ~~-10~~

@@ -104,22 +104,21 @@ static __be32 nfsacld_proc_setacl(struct
 		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);

fs/nfsd/nfs3acl.c +7 -9

@@ -95,22 +95,20 @@ static __be32 nfsd3_proc_setacl(struct s
 		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);

fs/nfsd/nfs4acl.c +8 -8

@@ -770,9 +770,6 @@ nfsd4_set_nfs4_acl(struct svc_rqst *rqst
 	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 *rqst
 	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:

Patches currently in stable-queue which might be from [email protected] are

queue-4.4/nfsd-check-permissions-when-setting-acls.patch