Subject: [nfsv4] Issues with read chunk description in RFC8166.

From: David Noveck <[email-masked]>
Date: Mon, 19 Nov 2018 13:55:38 -0500

I am now working on read chunk support and i can't seem to find a
description in RFc8166 about how to deal with a number of  erors or other
unusual situations that mght be generated by the client.   I have looked at
RFC8166 and, while I might have missed it, i''m not sure how these
situations are to be dealt with and whether the requests can/should be
rejected as errneous.

The first issue concerns a read chunk with a position field that is not
that of a DDP-eligible data item.   I would think these should result in a
CHUNK error but I can't find any statement to that effect.  Ignoring them
is possible but I can't see any statement to that effect either.

The other issues deal with cases in which the length of the read chunk is
diffeent from the length indicated in the request (i.e. by the length word
in the payload stream).  I have seen cases in which the Linux client
produuces a read chunk whose length is longer that the length indicated by
the length word.  I simply ignore the extra bytes, but I can't find any
statement justifyng that in RFC8166.   Also, if you read the description of
how read chunks work it would seem that the extra words are to be inserted
the payload stream, which would be a problem if there are ops after the
WRITE (e.g GETATTR).

The complementary problem, in which the read chunk's length is too small is
harder.  Luckily i haven't seen a client actually do that :-)   I imageine
would simply use the read chunk length although one could argue that the
read chunk should be zero-filled or that this should be an error.
 Unfrtunately I can't see where RFC8166 addresses this issue either.

---

From: Chuck Lever <[email-masked]>
Date: Mon, 19 Nov 2018 14:03:09 -0500

> On Nov 19, 2018, at 1:55 PM, David Noveck <[email-masked]> wrote:
> 
> I am now working on read chunk support and i can't seem to find a description in RFc8166 about how to deal with a number of  erors or other unusual situations that mght be generated by the client.   I have looked at RFC8166 and, while I might have missed it, i''m not sure how these situations are to be dealt with and whether the requests can/should be rejected as errneous.
> 
> The first issue concerns a read chunk with a position field that is not that of a DDP-eligible data item.   I would think these should result in a CHUNK error but I can't find any statement to that effect.  Ignoring them is possible but I can't see any statement to that effect either.  
> 
> The other issues deal with cases in which the length of the read chunk is diffeent from the length indicated in the request (i.e. by the length word in the payload stream).  I have seen cases in which the Linux client produuces a read chunk whose length is longer that the length indicated by the length word.  I simply ignore the extra bytes, but I can't find any statement justifyng that in RFC8166.   Also, if you read the description of how read chunks work it would seem that the extra words are to be inserted the payload stream, which would be a problem if there are ops after the WRITE (e.g GETATTR).
> 
> The complementary problem, in which the read chunk's length is too small is harder.  Luckily i haven't seen a client actually do that :-)   I imageine would simply use the read chunk length although one could argue that the read chunk should be zero-filled or that this should be an error.   Unfrtunately I can't see where RFC8166 addresses this issue either.

IMO ERR_CHUNK is the correct response in all these cases, and I don't
recall any discussion of these scenarios or text in 8166 that covers
them. You might file an errata if 8166 is truly missing this discussion.

If you see the Linux client sending too many bytes in a Read segment,
please capture that and send me a bug report and I will see that it
is addressed.

--
Chuck Lever

---

From: David Noveck <[email-masked]>
Date: Mon, 19 Nov 2018 15:06:57 -0500

> IMO ERR_CHUNK is the correct response in all these cases,

That's not unreasonable, but it is possible that, if these situations
happen, and current servers ignore them, we might not be able to make the
change in version one.

> and I don't
> them.

As I said, I didn't see them either.  I'll take a further look.   It seems
to me
that this discussion was missing in RFC5666, and the gaps were not noted
when RFC8166 was written, since there was not much active implementation
work at that tiime.   We had prototypes that worked together and situations
in
which the other partner made mistakes were not fully addressed by the
spec and nobody brought those gaps to your attention, unfortunately.

> You might file an errata if 8166 is truly missing this discussion.

I'll produce a summary of needed changes.   Once we have that, we can
decide
whether an errata or a supplementaryy rfc is best.   Given that so many
error
detection situations were not really addressed, we might have something too
big
for an errata.

> please capture that and send me a bug report and I will see that it

Will do that, but I have to work with the clients that exist today. Also,
if the client is
doing this, it is likely the server is ignoring it, so it is possible other
clients might be
doing this as well.  That makes changing this behavior kind of nightmarish.

On Mon, Nov 19, 2018 at 2:03 PM Chuck Lever <[email-masked]> wrote:

> > The other issues deal with cases in which the length of the read chunk
> is diffeent from the length indicated in the request (i.e. by the length
> word in the payload stream).  I have seen cases in which the Linux client
> > The complementary problem, in which the read chunk's length is too small
> is harder.  Luckily i haven't seen a client actually do that :-)   I
> imageine would simply use the read chunk length although one could argue
> that the read chunk should be zero-filled or that this should be an error.

---

From: Chuck Lever <[email-masked]>
Date: Mon, 19 Nov 2018 15:30:57 -0500

> On Nov 19, 2018, at 3:06 PM, David Noveck <[email-masked]> wrote:
> 
> As I said, I didn't see them either.  I'll take a further look.   It seems to me
> work at that tiime.   We had prototypes that worked together and situations in 
> I'll produce a summary of needed changes.   Once we have that, we can  decide 
> whether an errata or a supplementaryy rfc is best.   Given that so many error 
> detection situations were not really addressed, we might have something too big 
> for an errata. 
> 
> Will do that, but I have to work with the clients that exist today. Also, if the client is 
> doing this, it is likely the server is ignoring it, so it is possible other clients might be

Have you seen a client that sends an incorrect value in the position
field? If that's rare or hasn't occurred, then returning ERR_CHUNK
is not problematic.

For the second case, how many extra bytes are in the bad Read chunk?
Clients are allowed to include the XDR pad in the chunk, and servers
are required to ignore it.

--
Chuck Lever

---

From: David Noveck <[email-masked]>
Date: Mon, 19 Nov 2018 17:34:27 -0500

> field?

Never saw that.

> If that's rare or hasn't occurred, then returning ERR_CHUNK
> is not problematic.

True.

> For the second case, how many extra bytes are in the bad Read chunk?

sixteen.   The data length field was 0x4000 and the length in the read
chunk was
0x4010.   For all I know, it might be rare, but I suspect it isn't.

In any case, this is over three bytes.

On Mon, Nov 19, 2018 at 3:31 PM Chuck Lever <[email-masked]> wrote:

> > > The first issue concerns a read chunk with a position field that is
> not that of a DDP-eligible data item.   I would think these should result
> in a CHUNK error but I can't find any statement to that effect.  Ignoring
> them is possible but I can't see any statement to that effect either.
> > >
> > > The complementary problem, in which the read chunk's length is too
> small is harder.  Luckily i haven't seen a client actually do that :-)   I

---

From: Chuck Lever <[email-masked]>
Date: Mon, 19 Nov 2018 17:51:22 -0500

> On Nov 19, 2018, at 5:34 PM, David Noveck <[email-masked]> wrote:
> 
> sixteen.   The data length field was 0x4000 and the length in the read chunk was 

16 bytes is the size of an NFSv4 GETATTR operation. I suspect you
are using an old broken client that incorrectly includes the
trailing part of the compound in the Read chunk.

Servers these days would return an error because, depending on how
they handle those extra bytes, it could be ERR_CHUNK, or an NFSv4
error (BADXDR or OP_ILLEGAL).

I still believe returning an error in this scenario is the correct
thing to do.

--
Chuck Lever

---

From: David Noveck <[email-masked]>
Date: Mon, 19 Nov 2018 18:08:18 -0500

> 16 bytes is the size of an NFSv4 GETATTR operation.

> I suspect you

Perhaps but i also see the GETATTR (and it look correct) within
the payload stream.

> Servers these days would return an error because,
Is the implication that previously servers did not return an
error?  that matches our experience since we never saw a problem
(so far) in linux-to-linux testing.

> depending on how
> tthey handle those extra bytes, it could be ERR_CHUNK, or an NFSv4

That's assuming they treat theextra bytes as part of the paylodad stream,
which it seems to be they shouldn't do.

> I still believe returning an error in this scenario is the correct

I agree that the spec shoould say this, but will not switch over to that
behavior for a while.

---

From: Olga Kornievskaia <[email-masked]>
Date: Fri, 30 Nov 2018 16:16:17 -0500

> trailing part of the compound in the Read chunk.

I don't believe this is an issue with client sending incorrect values
(btw this is 4.18-rc2 client being tested, so somewhat old. I'll
update it soon). I'm going to try to convince Dave off-line about
this. But the discussion was useful to have.

> _______________________________________________
> nfsv4 mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/nfsv4

---

From: Chuck Lever <[email-masked]>
Date: Fri, 30 Nov 2018 16:29:08 -0500

Hi Olga-

> On Nov 30, 2018, at 4:16 PM, Olga Kornievskaia <[email-masked]> wrote:
> 
> update it soon).

This is a bug that was supposed to have been fixed years ago.
If this is still happening in v4.18-era kernels, it may have
been re-introduced or there's some other problem that needs
to be tracked down.

--
Chuck Lever