From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com ([209.132.183.28]:50194 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750816AbcGLUUu (ORCPT ); Tue, 12 Jul 2016 16:20:50 -0400 Date: Tue, 12 Jul 2016 16:20:32 -0400 From: "J. Bruce Fields" To: gregkh@linuxfoundation.org Cc: ben@decadent.org.uk, david@sinquin.eu, hch@infradead.org, viro@zeniv.linux.org.uk, stable@vger.kernel.org Subject: Re: FAILED: patch "[PATCH] nfsd: check permissions when setting ACLs" failed to apply to 3.14-stable tree Message-ID: <20160712202031.GC22259@pad> References: <1468276938249135@kroah.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1468276938249135@kroah.com> Sender: stable-owner@vger.kernel.org List-ID: On Mon, Jul 11, 2016 at 03:42:18PM -0700, gregkh@linuxfoundation.org wrote: > > The patch below does not apply to the 3.14-stable tree. > If someone wants it applied there, or to any other stable or longterm > tree, then please email the backport, including the original git commit > id to . Thanks, looks like only minor fixups are required, I'll send backports. --b. > > thanks, > > greg k-h > > ------------------ original commit in Linus's tree ------------------ > > From 999653786df6954a31044528ac3f7a5dadca08f4 Mon Sep 17 00:00:00 2001 > From: Ben Hutchings > Date: Wed, 22 Jun 2016 19:43:35 +0100 > Subject: [PATCH] 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 > 4ac7249e, which changed nfsd to use the set_acl inode operation directly > instead of going through xattr handlers. > > Reported-by: David Sinquin > [agreunba@redhat.com: use set_posix_acl] > Fixes: 4ac7249e > Cc: Christoph Hellwig > Cc: Al Viro > Cc: stable@vger.kernel.org > Signed-off-by: J. Bruce Fields > > diff --git a/fs/nfsd/nfs2acl.c b/fs/nfsd/nfs2acl.c > index 1580ea6fd64d..d08cd88155c7 100644 > --- a/fs/nfsd/nfs2acl.c > +++ b/fs/nfsd/nfs2acl.c > @@ -104,22 +104,21 @@ static __be32 nfsacld_proc_setacl(struct svc_rqst * rqstp, > 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); > diff --git a/fs/nfsd/nfs3acl.c b/fs/nfsd/nfs3acl.c > index 01df4cd7c753..0c890347cde3 100644 > --- a/fs/nfsd/nfs3acl.c > +++ b/fs/nfsd/nfs3acl.c > @@ -95,22 +95,20 @@ static __be32 nfsd3_proc_setacl(struct svc_rqst * rqstp, > 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); > diff --git a/fs/nfsd/nfs4acl.c b/fs/nfsd/nfs4acl.c > index 6adabd6049b7..71292a0d6f09 100644 > --- a/fs/nfsd/nfs4acl.c > +++ b/fs/nfsd/nfs4acl.c > @@ -770,9 +770,6 @@ nfsd4_set_nfs4_acl(struct svc_rqst *rqstp, struct svc_fh *fhp, > 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 *rqstp, struct svc_fh *fhp, > 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: >