nfsd: check permissions when setting ACLs (CVE-2016-XXXX)

4 files changed, 230 insertions, 0 deletions

diff --git a/debian/changelog b/debian/changelog
index 0c33506f43ee..467e056bf413 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -27,6 +27,7 @@ linux (4.6.2-2) UNRELEASED; urgency=medium
     - xt_compat_match_from_user doesn't need a retval
     - do compat validation via translate_table
     - introduce and use xt_copy_counters_from_user
+  * nfsd: check permissions when setting ACLs
 
  -- Ben Hutchings <ben@decadent.org.uk>  Thu, 16 Jun 2016 12:37:27 +0100
 
diff --git a/debian/patches/bugfix/all/nfsd-check-permissions-when-setting-acls.patch b/debian/patches/bugfix/all/nfsd-check-permissions-when-setting-acls.patch
new file mode 100644
index 000000000000..ce0aeb42176d
--- /dev/null
+++ b/debian/patches/bugfix/all/nfsd-check-permissions-when-setting-acls.patch
@@ -0,0 +1,145 @@
+From: Ben Hutchings <ben@decadent.org.uk>
+Date: Wed, 22 Jun 2016 19:43:35 +0100
+Subject: [PATCH] nfsd: check permissions when setting ACLs
+Origin: http://git.linux-nfs.org/?p=bfields/linux.git;a=commit;h=999653786df6954a31044528ac3f7a5dadca08f4
+
+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 | 20 ++++++++++----------
+ fs/nfsd/nfs3acl.c | 16 +++++++---------
+ fs/nfsd/nfs4acl.c | 16 ++++++++--------
+ 3 files changed, 25 insertions(+), 27 deletions(-)
+
+--- a/fs/nfsd/nfs2acl.c
++++ b/fs/nfsd/nfs2acl.c
+@@ -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);
+--- a/fs/nfsd/nfs3acl.c
++++ b/fs/nfsd/nfs3acl.c
+@@ -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);
+--- a/fs/nfsd/nfs4acl.c
++++ b/fs/nfsd/nfs4acl.c
+@@ -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:
diff --git a/debian/patches/bugfix/all/posix_acl-add-set_posix_acl.patch b/debian/patches/bugfix/all/posix_acl-add-set_posix_acl.patch
new file mode 100644
index 000000000000..152fc8ad670c
--- /dev/null
+++ b/debian/patches/bugfix/all/posix_acl-add-set_posix_acl.patch
@@ -0,0 +1,82 @@
+From: Andreas Gruenbacher <agruenba@redhat.com>
+Date: Wed, 22 Jun 2016 23:57:25 +0200
+Subject: [PATCH] posix_acl: Add set_posix_acl
+Origin: http://git.linux-nfs.org/?p=bfields/linux.git;a=commit;h=485e71e8fb6356c08c7fc6bcce4bf02c9a9a663f
+
+Factor out part of posix_acl_xattr_set into a common function that takes
+a posix_acl, which nfsd can also call.
+
+The prototype already exists in include/linux/posix_acl.h.
+
+Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
+Cc: stable@vger.kernel.org
+Cc: Christoph Hellwig <hch@infradead.org>
+Cc: Al Viro <viro@zeniv.linux.org.uk>
+Signed-off-by: J. Bruce Fields <bfields@redhat.com>
+[bwh: Backported to 4.6: posix_acl_xattr_set() parameters are different]
+---
+--- a/fs/posix_acl.c
++++ b/fs/posix_acl.c
+@@ -786,39 +786,43 @@ posix_acl_xattr_get(const struct xattr_h
+ 	return error;
+ }
+ 
+-static int
+-posix_acl_xattr_set(const struct xattr_handler *handler,
+-		    struct dentry *dentry, const char *name,
+-		    const void *value, size_t size, int flags)
++int
++set_posix_acl(struct inode *inode, int type, struct posix_acl *acl)
+ {
+-	struct inode *inode = d_backing_inode(dentry);
+-	struct posix_acl *acl = NULL;
+-	int ret;
+-
+ 	if (!IS_POSIXACL(inode))
+ 		return -EOPNOTSUPP;
+ 	if (!inode->i_op->set_acl)
+ 		return -EOPNOTSUPP;
+ 
+-	if (handler->flags == ACL_TYPE_DEFAULT && !S_ISDIR(inode->i_mode))
+-		return value ? -EACCES : 0;
++	if (type == ACL_TYPE_DEFAULT && !S_ISDIR(inode->i_mode))
++		return acl ? -EACCES : 0;
+ 	if (!inode_owner_or_capable(inode))
+ 		return -EPERM;
+ 
++	if (acl) {
++		int ret = posix_acl_valid(acl);
++		if (ret)
++			return ret;
++	}
++	return inode->i_op->set_acl(inode, acl, type);
++}
++EXPORT_SYMBOL(set_posix_acl);
++
++static int
++posix_acl_xattr_set(const struct xattr_handler *handler,
++		    struct dentry *dentry, const char *name,
++		    const void *value, size_t size, int flags)
++{
++	struct inode *inode = d_backing_inode(dentry);
++	struct posix_acl *acl = NULL;
++	int ret;
++
+ 	if (value) {
+ 		acl = posix_acl_from_xattr(&init_user_ns, value, size);
+ 		if (IS_ERR(acl))
+ 			return PTR_ERR(acl);
+-
+-		if (acl) {
+-			ret = posix_acl_valid(acl);
+-			if (ret)
+-				goto out;
+-		}
+ 	}
+-
+-	ret = inode->i_op->set_acl(inode, acl, handler->flags);
+-out:
++	ret = set_posix_acl(inode, handler->flags, acl);
+ 	posix_acl_release(acl);
+ 	return ret;
+ }
diff --git a/debian/patches/series b/debian/patches/series
index ad115dd32830..ec4eddb4f10d 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -131,6 +131,8 @@ bugfix/all/netfilter-ip6_tables-simplify-translate_compat_table.patch
 bugfix/all/netfilter-x_tables-xt_compat_match_from_user-doesn-t.patch
 bugfix/all/netfilter-x_tables-do-compat-validation-via-translat.patch
 bugfix/all/netfilter-x_tables-introduce-and-use-xt_copy_counter.patch
+bugfix/all/posix_acl-add-set_posix_acl.patch
+bugfix/all/nfsd-check-permissions-when-setting-acls.patch
 
 # ABI maintenance
 debian/mips-siginfo-fix-abi-change-in-4.6.2.patch