Re: [PATCH] NFSv4: Fix attribute length

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 2013-07-24 at 10:38 -0400, J. Bruce Fields wrote:
> On Wed, Jul 24, 2013 at 02:30:58PM +0000, Myklebust, Trond wrote:
> > On Wed, 2013-07-24 at 10:14 -0400, J. Bruce Fields wrote:
> > > From: "J. Bruce Fields" <bfields@xxxxxxxxxx>
> > > 
> > > The calculation of attribute length fields is too high by four because
> > > it incorrectly includes the length field itself.
> > > 
> > > This regression was introduced by
> > > b4a2cf76ab7c08628c62b2062dacefa496b59dfd "NFSv4: Fix a regression
> > > against the FreeBSD server" and causes OPENs to the Linux NFS server to
> > > fail with BADXDR errors (translated by the client into EIO).
> > > 
> > > Signed-off-by: J. Bruce Fields <bfields@xxxxxxxxxx>
> > > ---
> > >  fs/nfs/nfs4xdr.c |    2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > I thought you guys had automated testing against the Linux server?  How
> > > did this slip through into upstream?
> > > 
> > > --b.
> > > 
> > > diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> > > index c74d616..d6d6754 100644
> > > --- a/fs/nfs/nfs4xdr.c
> > > +++ b/fs/nfs/nfs4xdr.c
> > > @@ -1118,7 +1118,7 @@ static void encode_attrs(struct xdr_stream *xdr, const struct iattr *iap,
> > >  				len, ((char *)p - (char *)q) + 4);
> > >  		BUG();
> > >  	}
> > > -	len = (char *)p - (char *)q - (bmval_len << 2);
> > > +	len = (char *)p - (char *)q - (bmval_len + 1 << 2);
> > >  	*q++ = htonl(bmval0);
> > >  	*q++ = htonl(bmval1);
> > >  	if (bmval_len == 3)
> > 
> > Please see commit 4f3cc4809a98a165a9708b72b47de71643797bbd (NFSv4: Fix
> > brainfart in attribute length calculation) upstream.
> 
> Oh, good, thanks!
> 
> But I still don't understand how this made it into upstream in the first
> place.
> 
> Any NFSv4 testing at all against the Linux server would catch this, and
> I thought you had automated testing set up that you could run before
> submission.

Yes, however it was truly a brainfart: the patch was never tested
independently of the cleanup which removes the backfilling altogether.

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@xxxxxxxxxx
www.netapp.com
��.n��������+%������w��{.n�����{��w���jg��������ݢj����G�������j:+v���w�m������w�������h�����٥





[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux