Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210128223638.GE29887@fieldses.org> <95e5f9e4-76d4-08c4-ece3-35a10c06073b@vastdata.com>
 <cbc7115cc5d5aeb7ffb9e9b3880e453bf54ecbdb.camel@hammerspace.com>
 <20210129023527.GA11864@fieldses.org> <20210129025041.GA12151@fieldses.org>
 <7a078b4d22c8d769a42a0c2b47fd501479e47a7b.camel@hammerspace.com>
 <20210131215843.GA9273@fieldses.org> <20210203200756.GA30996@fieldses.org>
 <6dc98a594a21b86316bf77000dc620d6cca70be6.camel@hammerspace.com>
 <20210208220855.GA15116@fieldses.org> <20210211185444.GA6048@fieldses.org> <CADJHv_vH5Ocf8D4hXNSKc4PcdwJWCeXe+nvwiOqRWVQ7HOoNfg@mail.gmail.com>
In-Reply-To: <CADJHv_vH5Ocf8D4hXNSKc4PcdwJWCeXe+nvwiOqRWVQ7HOoNfg@mail.gmail.com>
From:   Anna Schumaker <schumakeranna@gmail.com>
Date:   Fri, 12 Mar 2021 10:43:41 -0500
Message-ID: <CAFX2JfnH1JWBWS5zm03mnvHDAfMAqmNKCH9fsjWu2u8sGvGO1g@mail.gmail.com>
Subject: Re: [PATCH] nfs: we don't support removing system.nfs4_acl
To:     Murphy Zhou <jencce.kernel@gmail.com>
Cc:     Trond Myklebust <trondmy@hammerspace.com>,
        "guy@vastdata.com" <guy@vastdata.com>,
        "linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>,
        "J. Bruce Fields" <bfields@fieldses.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Murphy,

On Wed, Mar 3, 2021 at 9:30 PM Murphy Zhou <jencce.kernel@gmail.com> wrote:
>
> Hi,
>
> On Fri, Feb 12, 2021 at 2:58 AM bfields@fieldses.org
> <bfields@fieldses.org> wrote:
> >
> > From: "J. Bruce Fields" <bfields@redhat.com>
> >
> > The contents of the system.nfs4_acl xattr are literally just the
> > xdr-encoded ACL attribute.  That attribute starts with a 4-byte integer
> > representing the number of ACEs in the ACL.  So even a zero-ACE ACL will
> > be at least 4 bytes.
> >
> > We've never actually bothered to sanity-check the ACL encoding that
> > userspace gives us.  The only problem that causes is that we return an
> > error that's probably wrong.  (The server will return BADXDR, which
> > we'll translate to EIO, when EINVAL would make more sense.)
> >
> > It's not much a problem in practice since the standard utilities give us
> > well-formed XDR.  The one case we're likely to see from userspace in
> > practice is a set of a zero-length xattr since that's how
> >
> >         removexattr(path, "system.nfs4_acl")
> >
> > is implemented.  It's worth trying to give a better error for that case.
> >
> > Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> > ---
> >  fs/nfs/nfs4proc.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > On Mon, Feb 08, 2021 at 05:08:55PM -0500, bfields@fieldses.org wrote:
> > > On Mon, Feb 08, 2021 at 07:31:38PM +0000, Trond Myklebust wrote:
> > > > OK. So you're not really saying that the SETATTR has a zero length
> > > > body, but that the ACL attribute in this case has a zero length body,
> > > > whereas in the 'empty acl' case, it is supposed to have a body
> > > > containing a zero-length nfsace4<> array. Fair enough.
> > >
> > > Yep!  I'll see if I can think of a helpful concise comment, and resend.
> >
> > Oops, forgot about this, here you go.--b.
> >
> > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > index 2f4679a62712..86e87f7d7686 100644
> > --- a/fs/nfs/nfs4proc.c
> > +++ b/fs/nfs/nfs4proc.c
> > @@ -5895,6 +5895,12 @@ static int __nfs4_proc_set_acl(struct inode *inode, const void *buf, size_t bufl
> >         unsigned int npages = DIV_ROUND_UP(buflen, PAGE_SIZE);
> >         int ret, i;
> >
> > +       /*
> > +        * We don't support removing system.nfs4_acl, and even a
> > +        * 0-length ACL needs at least 4 bytes for the number of ACEs:
> > +        */
> > +       if (buflen < 4)
> > +               return -EINVAL;
> >         if (!nfs4_server_supports_acls(server))
> >                 return -EOPNOTSUPP;
> >         if (npages > ARRAY_SIZE(pages))
> > --
> > 2.29.2
> >
>
> Has this queued up for the next RC ?

Yeah, I have this queued up for the next bugfixes pull request.

Anna
>
>
> Thanks,