Bug in xdr_copy_to_scratch???

Bug in xdr_copy_to_scratch???

[NeilBrown]

Hi Trond,

 I'm currently trying to track down the cause of some very
odd behaviour in readdir in openSUSE 11.4 (2.6.37.3 based).

I think it might be caused by xdr_copy_to_scratch not quite
behaving correctly.

In particular, when it has to copy into the scratch buffer
it only copies 'nbytes' bytes - which sounds reasonable but
isn't.  It should copy XDR_QUADLEN(nbytes) words.

In particular, nfs3_decode_dirent contains:

	p = xdr_inline_decode(xdr, entry->len + 8);
	if (unlikely(!p))
		goto out_overflow;
	entry->name = (const char *) p;
	p += XDR_QUADLEN(entry->len);
	entry->prev_cookie = entry->cookie;
	p = xdr_decode_hyper(p, &entry->cookie);


where the cookie needs all of those last few bytes which
we would only get by rounding nbytes up to a multiple of 4.


I haven't developed or tested a fix yet, but as it is clearly a bug,
I thought I would let you know before I call it a night.

Thanks,
NeilBrown

Re: Bug in xdr_copy_to_scratch???

[Trond Myklebust]

On Wed, 2011-03-16 at 21:36 +1100, NeilBrown wrote:
> Hi Trond,
> 
>  I'm currently trying to track down the cause of some very
> odd behaviour in readdir in openSUSE 11.4 (2.6.37.3 based).
> 
> I think it might be caused by xdr_copy_to_scratch not quite
> behaving correctly.
> 
> In particular, when it has to copy into the scratch buffer
> it only copies 'nbytes' bytes - which sounds reasonable but
> isn't.  It should copy XDR_QUADLEN(nbytes) words.
> 
> In particular, nfs3_decode_dirent contains:
> 
> 	p = xdr_inline_decode(xdr, entry->len + 8);
> 	if (unlikely(!p))
> 		goto out_overflow;
> 	entry->name = (const char *) p;
> 	p += XDR_QUADLEN(entry->len);
> 	entry->prev_cookie = entry->cookie;
> 	p = xdr_decode_hyper(p, &entry->cookie);
> 
> 
> where the cookie needs all of those last few bytes which
> we would only get by rounding nbytes up to a multiple of 4.
> 
> 
> I haven't developed or tested a fix yet, but as it is clearly a bug,
> I thought I would let you know before I call it a night.

Hi Neil,

I'm not sure I 100% agree that is a bug in xdr_copy_to_scratch(). I
think it is rather an artifact of the fact that xdr_inline_decode takes
a byte argument instead of a word argument, which means that combining
buffer lengths needs to done with care: my fault :-(.

To illustrate what I mean, consider the following snippet of xdr:

	p = xdr_decode_inline(xdr, len1 + len2);
	name1 = p;
	p += XDR_QUADLEN(len1);
	name2 = p;

No matter what we do in xdr_decode_inline(), there is no way we can
determine the true value of XDR_QUADLEN(len1)<<2 + XDR_QUADLEN(len2)<<2
if len1 and len2 are arbitrary buffer lengths.

So I'd argue that while we could fix this issue in xdr_copy_to_scratch
for this particular case, in reality nfs3_decode_dirent should not be
asking for a buffer of (entry->len + 8) bytes when it knows there is an
alignment issue due to the fact that the 8 cookie bytes follows the
string.

Cheers
  Trond
-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

Re: Bug in xdr_copy_to_scratch???

[NeilBrown]

On Wed, 16 Mar 2011 09:42:54 -0400 Trond Myklebust
<Trond.Myklebust@netapp.com> wrote:

> On Wed, 2011-03-16 at 21:36 +1100, NeilBrown wrote:
> > Hi Trond,
> > 
> >  I'm currently trying to track down the cause of some very
> > odd behaviour in readdir in openSUSE 11.4 (2.6.37.3 based).
> > 
> > I think it might be caused by xdr_copy_to_scratch not quite
> > behaving correctly.
> > 
> > In particular, when it has to copy into the scratch buffer
> > it only copies 'nbytes' bytes - which sounds reasonable but
> > isn't.  It should copy XDR_QUADLEN(nbytes) words.
> > 
> > In particular, nfs3_decode_dirent contains:
> > 
> > 	p = xdr_inline_decode(xdr, entry->len + 8);
> > 	if (unlikely(!p))
> > 		goto out_overflow;
> > 	entry->name = (const char *) p;
> > 	p += XDR_QUADLEN(entry->len);
> > 	entry->prev_cookie = entry->cookie;
> > 	p = xdr_decode_hyper(p, &entry->cookie);
> > 
> > 
> > where the cookie needs all of those last few bytes which
> > we would only get by rounding nbytes up to a multiple of 4.
> > 
> > 
> > I haven't developed or tested a fix yet, but as it is clearly a bug,
> > I thought I would let you know before I call it a night.
> 
> Hi Neil,
> 
> I'm not sure I 100% agree that is a bug in xdr_copy_to_scratch(). I
> think it is rather an artifact of the fact that xdr_inline_decode takes
> a byte argument instead of a word argument, which means that combining
> buffer lengths needs to done with care: my fault :-(.
> 
> To illustrate what I mean, consider the following snippet of xdr:
> 
> 	p = xdr_decode_inline(xdr, len1 + len2);
> 	name1 = p;
> 	p += XDR_QUADLEN(len1);
> 	name2 = p;
> 
> No matter what we do in xdr_decode_inline(), there is no way we can
> determine the true value of XDR_QUADLEN(len1)<<2 + XDR_QUADLEN(len2)<<2
> if len1 and len2 are arbitrary buffer lengths.
> 
> So I'd argue that while we could fix this issue in xdr_copy_to_scratch
> for this particular case, in reality nfs3_decode_dirent should not be
> asking for a buffer of (entry->len + 8) bytes when it knows there is an
> alignment issue due to the fact that the 8 cookie bytes follows the
> string.

Your argument certainly has some merit.
Probably the question to ask is "what sort of fix is most likely to
avoid such problems recurring in the future", and I cannot convince myself
that either is clearly better than the other on that basis.

I note that in 2.6.38 the readdir problem has been 'fixed' by the
introduction of decode_inline_filename3 and similar functions.
However there is still one place where xdr_decode_inline is used in
an unsafe way - see patch below.

We should probably submit a fix to 2.6.37-stable though.  For that it
is possibly simplest to tell xdr_decode_inline to round nbytes up to
a multiple of 4 - would you agree?

Also:  looking at xdr_copy_to_scratch again, is there any chance that
'cplen' could not be a multiple of 4?  I think not but I'm not
100% sure.  If it were not a multiple of 4, then the
__xdr_inline_decode call would do the wrong thing.


And finally - it bothers me a little bit that a call to xdr_copy_to_scratch
will corrupt the value returned by the previous call to
xdr_copy_to_scratch.  This is unlikely to be a problem as you
get at most one of those calls every 4096 bytes, and the previous
value will almost certainly have been used before the next value
is copied over it.  But it isn't clear what guarantees that it will
have been used, so a future change or extension to the protocol could
conceivably result in corruption happening in the scratch buffer.
Maybe it isn't worth worrying about.... not sure.

Thanks,
NeilBrown





Fix theoretical decoding problem in nfsv4 compound header decoding.

As all XDR encoded fields are rounded up to a multiple of 4
bytes in size, a string field followed by a number field could
have padding between the string and number.

If xdr_inline_decode is asked to decode the combined field
(by summing the lengths) it will normally work.  However if it
needs to copy the field into that 'scratch' space to avoid a
break between pages, it will only copy the requested number of bytes,
as it assumes any padding is at the end - so the number field will not be
fully copied.

So in decode_compound_hdr, call xdr_inline_decode twice instead of
risking an incorrect decode.

As this field is always near the start of a packet, the chance that it
will ever cross a page boundary and so cause a problem is vanishingly small,
but having 'wrong' code could set an example that might get copied. 
So fix it anyway.


Signed-off-by: NeilBrown <neilb@suse.de>

diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 4e2c168..c52b50c 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -2638,11 +2638,13 @@ static int decode_compound_hdr(struct xdr_stream *xdr, struct compound_hdr *hdr)
 	hdr->status = be32_to_cpup(p++);
 	hdr->taglen = be32_to_cpup(p);
 
-	p = xdr_inline_decode(xdr, hdr->taglen + 4);
+	p = xdr_inline_decode(xdr, hdr->taglen);
 	if (unlikely(!p))
 		goto out_overflow;
 	hdr->tag = (char *)p;
-	p += XDR_QUADLEN(hdr->taglen);
+	p = xdr_inline_decode(xdr, 4);
+	if (unlikely(!p))
+		goto out_overflow;
 	hdr->nops = be32_to_cpup(p);
 	if (unlikely(hdr->nops < 1))
 		return nfs4_stat_to_errno(hdr->status);

Re: Bug in xdr_copy_to_scratch???

[Trond Myklebust]

On Thu, 2011-03-17 at 09:38 +1100, NeilBrown wrote:
> On Wed, 16 Mar 2011 09:42:54 -0400 Trond Myklebust
> <Trond.Myklebust@netapp.com> wrote:
> 
> > On Wed, 2011-03-16 at 21:36 +1100, NeilBrown wrote:
> > > Hi Trond,
> > > 
> > >  I'm currently trying to track down the cause of some very
> > > odd behaviour in readdir in openSUSE 11.4 (2.6.37.3 based).
> > > 
> > > I think it might be caused by xdr_copy_to_scratch not quite
> > > behaving correctly.
> > > 
> > > In particular, when it has to copy into the scratch buffer
> > > it only copies 'nbytes' bytes - which sounds reasonable but
> > > isn't.  It should copy XDR_QUADLEN(nbytes) words.
> > > 
> > > In particular, nfs3_decode_dirent contains:
> > > 
> > > 	p = xdr_inline_decode(xdr, entry->len + 8);
> > > 	if (unlikely(!p))
> > > 		goto out_overflow;
> > > 	entry->name = (const char *) p;
> > > 	p += XDR_QUADLEN(entry->len);
> > > 	entry->prev_cookie = entry->cookie;
> > > 	p = xdr_decode_hyper(p, &entry->cookie);
> > > 
> > > 
> > > where the cookie needs all of those last few bytes which
> > > we would only get by rounding nbytes up to a multiple of 4.
> > > 
> > > 
> > > I haven't developed or tested a fix yet, but as it is clearly a bug,
> > > I thought I would let you know before I call it a night.
> > 
> > Hi Neil,
> > 
> > I'm not sure I 100% agree that is a bug in xdr_copy_to_scratch(). I
> > think it is rather an artifact of the fact that xdr_inline_decode takes
> > a byte argument instead of a word argument, which means that combining
> > buffer lengths needs to done with care: my fault :-(.
> > 
> > To illustrate what I mean, consider the following snippet of xdr:
> > 
> > 	p = xdr_decode_inline(xdr, len1 + len2);
> > 	name1 = p;
> > 	p += XDR_QUADLEN(len1);
> > 	name2 = p;
> > 
> > No matter what we do in xdr_decode_inline(), there is no way we can
> > determine the true value of XDR_QUADLEN(len1)<<2 + XDR_QUADLEN(len2)<<2
> > if len1 and len2 are arbitrary buffer lengths.
> > 
> > So I'd argue that while we could fix this issue in xdr_copy_to_scratch
> > for this particular case, in reality nfs3_decode_dirent should not be
> > asking for a buffer of (entry->len + 8) bytes when it knows there is an
> > alignment issue due to the fact that the 8 cookie bytes follows the
> > string.
> 
> Your argument certainly has some merit.
> Probably the question to ask is "what sort of fix is most likely to
> avoid such problems recurring in the future", and I cannot convince myself
> that either is clearly better than the other on that basis.
> 
> I note that in 2.6.38 the readdir problem has been 'fixed' by the
> introduction of decode_inline_filename3 and similar functions.
> However there is still one place where xdr_decode_inline is used in
> an unsafe way - see patch below.
> 
> We should probably submit a fix to 2.6.37-stable though.  For that it
> is possibly simplest to tell xdr_decode_inline to round nbytes up to
> a multiple of 4 - would you agree?

The only case that seems affected is nfs3_decode_dirent(). The other
decode_dirent cases are all OK AFAICS.

> Also:  looking at xdr_copy_to_scratch again, is there any chance that
> 'cplen' could not be a multiple of 4?  I think not but I'm not
> 100% sure.  If it were not a multiple of 4, then the
> __xdr_inline_decode call would do the wrong thing.

If it is not a multiple of 4, then that would mean we must have decoded
the preceding xdr-encoded buffer incorrectly.

> And finally - it bothers me a little bit that a call to xdr_copy_to_scratch
> will corrupt the value returned by the previous call to
> xdr_copy_to_scratch.  This is unlikely to be a problem as you
> get at most one of those calls every 4096 bytes, and the previous
> value will almost certainly have been used before the next value
> is copied over it.  But it isn't clear what guarantees that it will
> have been used, so a future change or extension to the protocol could
> conceivably result in corruption happening in the scratch buffer.
> Maybe it isn't worth worrying about.... not sure.

The long term solution is to switch to using words instead of bytes as
the argument to xdr_inline_decode(). If we force people to use
XDR_QUADLEN() on non-word sized arguments, then that will avoid any
confusion.

> 
> Fix theoretical decoding problem in nfsv4 compound header decoding.
> 
> As all XDR encoded fields are rounded up to a multiple of 4
> bytes in size, a string field followed by a number field could
> have padding between the string and number.
> 
> If xdr_inline_decode is asked to decode the combined field
> (by summing the lengths) it will normally work.  However if it
> needs to copy the field into that 'scratch' space to avoid a
> break between pages, it will only copy the requested number of bytes,
> as it assumes any padding is at the end - so the number field will not be
> fully copied.
> 
> So in decode_compound_hdr, call xdr_inline_decode twice instead of
> risking an incorrect decode.
> 
> As this field is always near the start of a packet, the chance that it
> will ever cross a page boundary and so cause a problem is vanishingly small,
> but having 'wrong' code could set an example that might get copied. 
> So fix it anyway.
> 
> 
> Signed-off-by: NeilBrown <neilb@suse.de>
> 
> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> index 4e2c168..c52b50c 100644
> --- a/fs/nfs/nfs4xdr.c
> +++ b/fs/nfs/nfs4xdr.c
> @@ -2638,11 +2638,13 @@ static int decode_compound_hdr(struct xdr_stream *xdr, struct compound_hdr *hdr)
>  	hdr->status = be32_to_cpup(p++);
>  	hdr->taglen = be32_to_cpup(p);
>  
> -	p = xdr_inline_decode(xdr, hdr->taglen + 4);
> +	p = xdr_inline_decode(xdr, hdr->taglen);
>  	if (unlikely(!p))
>  		goto out_overflow;
>  	hdr->tag = (char *)p;
> -	p += XDR_QUADLEN(hdr->taglen);
> +	p = xdr_inline_decode(xdr, 4);
> +	if (unlikely(!p))
> +		goto out_overflow;
>  	hdr->nops = be32_to_cpup(p);
>  	if (unlikely(hdr->nops < 1))
>  		return nfs4_stat_to_errno(hdr->status);

There is a similar instance in fs/nfsd/nfs4callback.c: see
decode_cb_compound4res().

Might as well fix both in the same way.

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

Re: Bug in xdr_copy_to_scratch???

[Trond Myklebust]

On Thu, 2011-03-17 at 09:38 +1100, NeilBrown wrote:
> We should probably submit a fix to 2.6.37-stable though.  For that it
> is possibly simplest to tell xdr_decode_inline to round nbytes up to
> a multiple of 4 - would you agree?

How about the following fix for 2.6.37 stable?

8<-------------------------------------------------------------------------

Re: Bug in xdr_copy_to_scratch???

[NeilBrown]

On Thu, 17 Mar 2011 14:01:05 -0400 Trond Myklebust
<Trond.Myklebust@netapp.com> wrote:

> On Thu, 2011-03-17 at 09:38 +1100, NeilBrown wrote:
> > We should probably submit a fix to 2.6.37-stable though.  For that it
> > is possibly simplest to tell xdr_decode_inline to round nbytes up to
> > a multiple of 4 - would you agree?
> 
> How about the following fix for 2.6.37 stable?

That is good for NFSv3, but NFSv2 has the same problem.  Code fragment is
	p = xdr_inline_decode(xdr, entry->len + 4);
	if (unlikely(!p))
		goto out_overflow;
	entry->name	  = (const char *) p;
	p		 += XDR_QUADLEN(entry->len);
	entry->prev_cookie	  = entry->cookie;
	entry->cookie	  = ntohl(*p++);

so again we have the cookie after the name and they are decoded together.

This is what I have committed to openSUSE 11.3 for the next update.
It may not match the long-term preferred direction, but it is simple and
obviously covers all cases.

NeilBrown

From: NeilBrown <neilb@suse.de>
Subject: Fix cookie decoding problem in NFS
Patch-mainline: no
References: bnc#678123

nfs3proc decode_dirent asks xdr_inline_decode to return a name
buffer and the cookie in a single request.
There could be padding between these, but if xdr_inline_decode
calls xdr_copy_to_scratch, it will assume the padding is at the end.

So in xdr_copy_to_scratch, round up 'nbytes' to we make don't
fail to return useful data.

Acked-by: NeilBrown <neilb@suse.de>
Signed-off-by: Neil Brown <neilb@suse.de>

---
 net/sunrpc/xdr.c |    5 +++++
 1 file changed, 5 insertions(+)

--- linux-2.6.37-openSUSE-11.4.orig/net/sunrpc/xdr.c
+++ linux-2.6.37-openSUSE-11.4/net/sunrpc/xdr.c
@@ -673,6 +673,11 @@ static __be32 *xdr_copy_to_scratch(struc
 	void *cpdest = xdr->scratch.iov_base;
 	size_t cplen = (char *)xdr->end - (char *)xdr->p;
 
+	/* some callers assume we return the full rounded-up
+	 * number of bytes.
+	 */
+	nbytes = XDR_QUADLEN(nbytes)*4;
+
 	if (nbytes > xdr->scratch.iov_len)
 		return NULL;
 	memcpy(cpdest, xdr->p, cplen);

Re: Bug in xdr_copy_to_scratch???

[Trond Myklebust]

On Fri, 2011-03-18 at 10:16 +1100, NeilBrown wrote:
> On Thu, 17 Mar 2011 14:01:05 -0400 Trond Myklebust
> <Trond.Myklebust@netapp.com> wrote:
> 
> > On Thu, 2011-03-17 at 09:38 +1100, NeilBrown wrote:
> > > We should probably submit a fix to 2.6.37-stable though.  For that it
> > > is possibly simplest to tell xdr_decode_inline to round nbytes up to
> > > a multiple of 4 - would you agree?
> > 
> > How about the following fix for 2.6.37 stable?
> 
> That is good for NFSv3, but NFSv2 has the same problem.  Code fragment is
> 	p = xdr_inline_decode(xdr, entry->len + 4);
> 	if (unlikely(!p))
> 		goto out_overflow;
> 	entry->name	  = (const char *) p;
> 	p		 += XDR_QUADLEN(entry->len);
> 	entry->prev_cookie	  = entry->cookie;
> 	entry->cookie	  = ntohl(*p++);
> 
> so again we have the cookie after the name and they are decoded together.

Fair enough. I'll fix that one too.

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com

Re: Bug in xdr_copy_to_scratch???

[Trond Myklebust]

On Thu, 2011-03-17 at 19:22 -0400, Trond Myklebust wrote:
> On Fri, 2011-03-18 at 10:16 +1100, NeilBrown wrote:
> > On Thu, 17 Mar 2011 14:01:05 -0400 Trond Myklebust
> > <Trond.Myklebust@netapp.com> wrote:
> > 
> > > On Thu, 2011-03-17 at 09:38 +1100, NeilBrown wrote:
> > > > We should probably submit a fix to 2.6.37-stable though.  For that it
> > > > is possibly simplest to tell xdr_decode_inline to round nbytes up to
> > > > a multiple of 4 - would you agree?
> > > 
> > > How about the following fix for 2.6.37 stable?
> > 
> > That is good for NFSv3, but NFSv2 has the same problem.  Code fragment is
> > 	p = xdr_inline_decode(xdr, entry->len + 4);
> > 	if (unlikely(!p))
> > 		goto out_overflow;
> > 	entry->name	  = (const char *) p;
> > 	p		 += XDR_QUADLEN(entry->len);
> > 	entry->prev_cookie	  = entry->cookie;
> > 	entry->cookie	  = ntohl(*p++);
> > 
> > so again we have the cookie after the name and they are decoded together.
> 
> Fair enough. I'll fix that one too.
> 

Here is the result.

BTW, those are the only cases we care about in -stable. There are no
other callers of xdr_set_scratch_buffer().

8<---------------------------------------------------------------------------------

Re: Bug in xdr_copy_to_scratch???

[NeilBrown]

On Thu, 17 Mar 2011 19:27:53 -0400 Trond Myklebust
<Trond.Myklebust@netapp.com> wrote:

> On Thu, 2011-03-17 at 19:22 -0400, Trond Myklebust wrote:
> > On Fri, 2011-03-18 at 10:16 +1100, NeilBrown wrote:
> > > On Thu, 17 Mar 2011 14:01:05 -0400 Trond Myklebust
> > > <Trond.Myklebust@netapp.com> wrote:
> > > 
> > > > On Thu, 2011-03-17 at 09:38 +1100, NeilBrown wrote:
> > > > > We should probably submit a fix to 2.6.37-stable though.  For that it
> > > > > is possibly simplest to tell xdr_decode_inline to round nbytes up to
> > > > > a multiple of 4 - would you agree?
> > > > 
> > > > How about the following fix for 2.6.37 stable?
> > > 
> > > That is good for NFSv3, but NFSv2 has the same problem.  Code fragment is
> > > 	p = xdr_inline_decode(xdr, entry->len + 4);
> > > 	if (unlikely(!p))
> > > 		goto out_overflow;
> > > 	entry->name	  = (const char *) p;
> > > 	p		 += XDR_QUADLEN(entry->len);
> > > 	entry->prev_cookie	  = entry->cookie;
> > > 	entry->cookie	  = ntohl(*p++);
> > > 
> > > so again we have the cookie after the name and they are decoded together.
> > 
> > Fair enough. I'll fix that one too.
> > 
> 
> Here is the result.
> 
> BTW, those are the only cases we care about in -stable. There are no
> other callers of xdr_set_scratch_buffer().

Yes, I agree.  That is a good fix - thanks.

Reviewed-by: NeilBrown <neilb@suse.de>


Thanks,
NeilBrown

> 
> 8<---------------------------------------------------------------------------------
> >From f71825a9002d4008a5b2ac947bc03b9d7c788557 Mon Sep 17 00:00:00 2001
> From: Trond Myklebust <Trond.Myklebust@netapp.com>
> Date: Thu, 17 Mar 2011 13:42:31 -0400
> Subject: [PATCH] NFS: Fix a decoding problem in nfs3_decode_dirent
> 
> When we decode a filename followed by an 8-byte cookie, we need to
> consider the fact that the filename and cookie are 32-bit word aligned.
> Presently, we may end up copying insufficient amounts of data when
> xdr_inline_decode() needs to invoke xdr_copy_to_scratch to deal
> with a page boundary.
> 
> The following patch fixes the issue by first decoding the filename, and
> then decoding the cookie.
> 
> Reported-by: Neil Brown <neilb@suse.de>
> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> ---
>  fs/nfs/nfs2xdr.c |    6 ++++--
>  fs/nfs/nfs3xdr.c |    6 ++++--
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/nfs/nfs2xdr.c b/fs/nfs/nfs2xdr.c
> index b382a1b..33a038d 100644
> --- a/fs/nfs/nfs2xdr.c
> +++ b/fs/nfs/nfs2xdr.c
> @@ -477,11 +477,13 @@ nfs_decode_dirent(struct xdr_stream *xdr, struct nfs_entry *entry, struct nfs_se
>  	entry->ino	  = ntohl(*p++);
>  	entry->len	  = ntohl(*p++);
>  
> -	p = xdr_inline_decode(xdr, entry->len + 4);
> +	p = xdr_inline_decode(xdr, entry->len);
>  	if (unlikely(!p))
>  		goto out_overflow;
>  	entry->name	  = (const char *) p;
> -	p		 += XDR_QUADLEN(entry->len);
> +	p = xdr_inline_decode(xdr, 4);
> +	if (unlikely(!p))
> +		goto out_overflow;
>  	entry->prev_cookie	  = entry->cookie;
>  	entry->cookie	  = ntohl(*p++);
>  
> diff --git a/fs/nfs/nfs3xdr.c b/fs/nfs/nfs3xdr.c
> index ba91236..dcd934f 100644
> --- a/fs/nfs/nfs3xdr.c
> +++ b/fs/nfs/nfs3xdr.c
> @@ -614,11 +614,13 @@ nfs3_decode_dirent(struct xdr_stream *xdr, struct nfs_entry *entry, struct nfs_s
>  	p = xdr_decode_hyper(p, &entry->ino);
>  	entry->len  = ntohl(*p++);
>  
> -	p = xdr_inline_decode(xdr, entry->len + 8);
> +	p = xdr_inline_decode(xdr, entry->len);
>  	if (unlikely(!p))
>  		goto out_overflow;
>  	entry->name = (const char *) p;
> -	p += XDR_QUADLEN(entry->len);
> +	p = xdr_inline_decode(xdr, 8);
> +	if (unlikely(!p))
> +		goto out_overflow;
>  	entry->prev_cookie = entry->cookie;
>  	p = xdr_decode_hyper(p, &entry->cookie);
>