[linux] 01/01: nfsd: check permissions when setting ACLs (CVE-2016-XXXX)
debian-kernel at lists.debian.org
debian-kernel at lists.debian.org
Fri Jun 24 22:56:24 UTC 2016
This is an automated email from the git hooks/post-receive script.
benh pushed a commit to branch sid
in repository linux.
commit 12183bf924db33672103a19328a4fa2ea89cec44
Author: Ben Hutchings <ben at decadent.org.uk>
Date: Sat Jun 25 00:56:15 2016 +0200
nfsd: check permissions when setting ACLs (CVE-2016-XXXX)
---
debian/changelog | 1 +
.../nfsd-check-permissions-when-setting-acls.patch | 145 +++++++++++++++++++++
.../bugfix/all/posix_acl-add-set_posix_acl.patch | 82 ++++++++++++
debian/patches/series | 2 +
4 files changed, 230 insertions(+)
diff --git a/debian/changelog b/debian/changelog
index 0c33506..467e056 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 at 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 0000000..ce0aeb4
--- /dev/null
+++ b/debian/patches/bugfix/all/nfsd-check-permissions-when-setting-acls.patch
@@ -0,0 +1,145 @@
+From: Ben Hutchings <ben at 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 at sinquin.eu>
+[agreunba at redhat.com: use set_posix_acl]
+Fixes: 4ac7249e
+Cc: Christoph Hellwig <hch at infradead.org>
+Cc: Al Viro <viro at zeniv.linux.org.uk>
+Cc: stable at vger.kernel.org
+Signed-off-by: J. Bruce Fields <bfields at 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 0000000..152fc8a
--- /dev/null
+++ b/debian/patches/bugfix/all/posix_acl-add-set_posix_acl.patch
@@ -0,0 +1,82 @@
+From: Andreas Gruenbacher <agruenba at 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 at redhat.com>
+Cc: stable at vger.kernel.org
+Cc: Christoph Hellwig <hch at infradead.org>
+Cc: Al Viro <viro at zeniv.linux.org.uk>
+Signed-off-by: J. Bruce Fields <bfields at 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 ad115dd..ec4eddb 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
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/kernel/linux.git
More information about the Kernel-svn-changes
mailing list