On Fri, 2020-12-04 at 20:34 +0200, Dan Aloni wrote:
> When receiving pages data, return value 'ret' when positive includes
> `buf->page_base`, so we should subtract it similarly to how it is
> done
> for `offset`.
>
> This was discovered on the very rare cases where the server returned
> a
> chunk of bytes that when added to the already received amount of
> bytes
> for the pages happened to match the current `recv.len`, for example
> on this case:
>
> buf->page_base : 258356
> actually received from socket: 1740
> ret : 260096
> want : 260096
>
> In this case neither of the two 'if ... goto out' trigger, and we
> continue to tail parsing.
>
> Worth to mention that the ensuing EMSGSIZE from the continued
> execution of
> `xs_read_xdr_buf` may be observed by an application due to 4
> superfluous
> bytes being added to the pages data.
>
> Fixes: 277e4ab7d53 ("SUNRPC: Simplify TCP receive code by switching
> to
> using iterators")
> Signed-off-by: Dan Aloni <dan@xxxxxxxxxxxx>
> ---
> net/sunrpc/xprtsock.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> index 7090bbee0ec5..42b680a60d38 100644
> --- a/net/sunrpc/xprtsock.c
> +++ b/net/sunrpc/xprtsock.c
> @@ -436,7 +436,7 @@ xs_read_xdr_buf(struct socket *sock, struct
> msghdr *msg, int flags,
> offset += ret - buf->page_base;
> if (offset == count || msg->msg_flags &
> (MSG_EOR|MSG_TRUNC))
> goto out;
> - if (ret != want)
> + if (ret - buf->page_base != want)
> goto out;
> seek = 0;
> } else {
Ouch... Well spotted!
Hmm... I think we want to just subtract out the buf->page_base from the
value of 'ret' after we call xs_flush_bvec() and then adjust the
calculation of 'offset' in the next line. That's more efficient.
Can you please resend with that change?
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@xxxxxxxxxxxxxxx
