Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 20 Jul 2023 19:15:19 -0400
From:   Chuck Lever <cel@kernel.org>
To:     NeilBrown <neilb@suse.de>
Cc:     Jeff Layton <jlayton@kernel.org>,
        Chuck Lever <chuck.lever@oracle.com>,
        Olga Kornievskaia <kolga@netapp.com>,
        Dai Ngo <Dai.Ngo@oracle.com>, Tom Talpey <tom@talpey.com>,
        Boyang Xue <bxue@redhat.com>, linux-nfs@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 0/2] nfsd: sanely handle inabilty to fetch pre/post
 attributes
Message-ID: <ZLnAB8Nfy/hPBhFl@manet.1015granger.net>
References: <20230720-bz2223560-v2-0-070aaf2660b7@kernel.org>
 <168988936713.11078.5407820394334916284@noble.neil.brown.name>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <168988936713.11078.5407820394334916284@noble.neil.brown.name>
Precedence: bulk

On Fri, Jul 21, 2023 at 07:42:47AM +1000, NeilBrown wrote:
> On Fri, 21 Jul 2023, Jeff Layton wrote:
> > Boyang reported tripping the BUG_ON in set_change_info. While we
> > couldn't confirm it, one way this could happen would be for nfsd_lookup
> > to succeed and then for fh_fill_both_attrs to fail.
> > 
> > This patchset attempts to (sanely) fix this, usually by aborting the
> > operation if fetching the pre attributes fails. Post-op attribute fetch
> > handling is more difficult to deal with however since we've already done
> > the operation, so this has it just fudge the change_info4 if that
> > occurs.
> 
> I think both v3 and v4 allow a reply that says "the operation was a
> success but there are no post-op attrs".  With v4 you can say "there is
> no change-attr, but here are some other attrs".  I think.

If the protocols enable NFSD to avoid returning made-up values, I'm
all for it.


> Our xdr-encoding doesn't make that easy, but it is just a "simple matter
> of coding".  If you think it is worth it.
> 
> NeilBrown
> 
> 
> > 
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> > Changes in v2:
> > - make fh_fill_*_attrs return an error and have the callers handle it
> > - rework of set_change_info, to better handle missing pre/post attrs
> > 
> > ---
> > Jeff Layton (2):
> >       nfsd: handle failure to collect pre/post-op attrs more sanely
> >       nfsd: remove unsafe BUG_ON from set_change_info
> > 
> >  fs/nfsd/nfs3proc.c |  4 +++-
> >  fs/nfsd/nfs4proc.c | 45 +++++++++++++++++++++++++++++++++------
> >  fs/nfsd/nfsfh.c    | 26 ++++++++++++++---------
> >  fs/nfsd/nfsfh.h    |  6 +++---
> >  fs/nfsd/vfs.c      | 62 ++++++++++++++++++++++++++++++++++--------------------
> >  fs/nfsd/xdr4.h     | 11 ----------
> >  6 files changed, 100 insertions(+), 54 deletions(-)
> > ---
> > base-commit: 070f391ca4d48e1750ee6108eb44f751a9e9448e
> > change-id: 20230720-bz2223560-9c4690a8217b
> > 
> > Best regards,
> > -- 
> > Jeff Layton <jlayton@kernel.org>
> > 
> > 
>