Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752360AbcCRG3Z (ORCPT ); Fri, 18 Mar 2016 02:29:25 -0400 Received: from mx2.suse.de ([195.135.220.15]:57694 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750721AbcCRG3R (ORCPT ); Fri, 18 Mar 2016 02:29:17 -0400 From: NeilBrown To: Sergio Gelato , Jiri Slaby Date: Fri, 18 Mar 2016 17:29:08 +1100 Cc: stable@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 3.12 01/58] nfsd: fix problem with setting ACL on directories In-Reply-To: <20160316122404.GA22901@hanuman.astro.su.se> References: <377b71e18f20d69b0df301ce7040554f40ba9651.1458125909.git.jslaby@suse.cz> <20160316122404.GA22901@hanuman.astro.su.se> User-Agent: Notmuch/0.20.2 (http://notmuchmail.org) Emacs/24.5.1 (x86_64-suse-linux-gnu) Message-ID: <87oaace1u3.fsf@notabene.neil.brown.name> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2897 Lines: 85 --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable On Wed, Mar 16 2016, Sergio Gelato wrote: > * Jiri Slaby [2016-03-16 11:58:46 +0100]: >> From: NeilBrown >>=20 >> 3.12-stable review patch. If anyone has any objections, please let me k= now. > >> If a non-inherited ACL is set on a directory, nfsd will try to set the P= osix >> default ACL to NULL. This gets converted to "" by generic_setxattr(). >> As "" is not a valid posix acl attribute value, this results in an error. >>=20 >> So instead of setting the xattr to NULL, remove it. > > There is similar code in nfsd_set_posix_acl() further down in the same so= urce > file which skips the vfs_removexattr() call for default ACLs on non-direc= tories > (there shouldn't be too many of these) and ignores ENODATA returns from > vfs_removexattr(). Are these precautions guaranteed to be unnecessary her= e in > set_nfsv4_acl_one() ? Those are not precautions, they are optimisations. The vfs_removexattr call would not be harmful, but would be unnecessary. So we don't need to worry about that for set_nfsv4_acl_one - we only apply optimisations to -stable if they are very significant. Thanks, NeilBrown > >> Fixes: ba1816b40a ("nfsd: fix NFS regression") >> Signed-off-by: NeilBrown >> Cc: Sergio Gelato >> Signed-off-by: Jiri Slaby >> --- >> fs/nfsd/vfs.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >>=20 >> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c >> index fafac65804d6..e5f146c7c871 100644 >> --- a/fs/nfsd/vfs.c >> +++ b/fs/nfsd/vfs.c >> @@ -510,7 +510,7 @@ set_nfsv4_acl_one(struct dentry *dentry, struct posi= x_acl *pacl, char *key) >> int error =3D 0; >>=20=20 >> if (!pacl) >> - return vfs_setxattr(dentry, key, NULL, 0, 0); >> + return vfs_removexattr(dentry, key); >>=20=20 >> buflen =3D posix_acl_xattr_size(pacl->a_count); >> buf =3D kmalloc(buflen, GFP_KERNEL); >> --=20 >> 2.7.3 >>=20 --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJW66A0AAoJEDnsnt1WYoG5UOYP/1AwOtomrhHqh4nmPsvFD0LZ f5PtmDW1+DA7OdyPAodTpm92qYppO8a2tGxXRl9BXBp7nnXk7od5Hxils8IouLc5 TAJXkFnbuj5ZdjdU+cxjV7pS/F/UCmE4faBmlJEhHpqG6JZXRa4x2kXBtjYyofI5 VKqb5nx7Xy/BUv6RwRYrAGkVf8pxyDaJt0MHlDkSJ+iQmjnGlbKTzZtjpGXHLIZu BN7Zry+wITUtabrp2XT0Itn99eh9ESoxOCwFhjY2CY3jbORFYojOq/udSIaL2e5S 9Er61G3kAYnQaO5B545HeFgZqh4HP8Kh4usMZpPSyAJ2mkayQ13pYtS1OQceobHh 4d/OL3N9a0W6BHYFIex/oY/F8Fnj5Ye0dEYrq4V82B44eCGgj8zsNj4xZBJkx5mu GI/+Mpja3pjkJ4g1Kh6Ygyu1YKb4hf+vKiLcbi7XoiQT1ry8Gfd46XNQxlHG3qJ7 Ww1suLqf7jigk9m22BYCZjJ+MwWz2QnEwyrdjzt/CT/f+IHoiBx8RhQlUFGABxvs INTln7aI/Ks5DLQmDpvnEL+0DuNJoN6f8npb50ReImCLCOGHwS7KmBqpBAdQzREN 8KW8gZounANpVqDX+kQfWJwsDttG/QEHGWkjRzvGSJqmP4xmqrhgY6LdZ3Vl3Yt6 TgJB/dF6MPiiKv2JHNj5 =mOia -----END PGP SIGNATURE----- --=-=-=--