nfsd: check permissions when setting ACLs (CVE-2016-XXXX)
This commit is contained in:
Ben Hutchings
parent
9fd7bbdf1a
commit
12183bf924
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Reference in New Issue