Re: [PATCH] SUNRPC: Fix another issue with MIC buffer space

[Chuck Lever <chuck.lever@oracle.com>]

> On Nov 15, 2019, at 9:41 AM, Chuck Lever <chuck.lever@oracle.com> wrote:
> 
> 
> 
>> On Nov 15, 2019, at 9:35 AM, Benjamin Coddington <bcodding@redhat.com> wrote:
>> 
>> On 15 Nov 2019, at 8:39, Chuck Lever wrote:
>> 
>>> xdr_shrink_pagelen() BUG's when @len is larger than buf->page_len.
>>> This can happen when xdr_buf_read_mic() is given an xdr_buf with
>>> a small page array (like, only a few bytes).
>> 
>> Hi Chuck,
>> 
>> Seems like a bug in xdr_buf_read_mic to me, but I'm not seeing how this can
>> happen.. unless perhaps xdr->page_len is 0?  Or maybe xdr_shift_buf has bug?
> 
> rpc_prepare_reply_pages() sets buf->page_len to the args->count of the
> NFS READ request. For really small READs, this can be 2, or 12, or
> anything smaller than the MIC length.
> 
> 
>> I'd prefer to keep the BUG_ON.
> 
> Linus would prefer not to. :-)
> 
> 
>> How can I reproduce it?
> 
> I've been using the git regression suite with NFSv4.1 and krb5i.
> I run it with 12 threads.

And I enable disconnect injection. Yer basic torture test.


>> diff --git a/net/sunrpc/xdr.c b/net/sunrpc/xdr.c
>> index 14ba9e72a204..71d754fc780e 100644
>> --- a/net/sunrpc/xdr.c
>> +++ b/net/sunrpc/xdr.c
>> @@ -1262,6 +1262,8 @@ int xdr_buf_read_mic(struct xdr_buf *buf, struct xdr_netobj *mic, unsigned int o
>>       if (offset < boundary && (offset + mic->len) > boundary)
>>               xdr_shift_buf(buf, boundary - offset);
>> 
>> +       trace_printk("boundary %d, offset %d, page_len %d\n", boundary, offset, buf->page_len);
>> +
>>       /* Is the mic partially in the pages? */
>>       boundary += buf->page_len;
>>       if (offset < boundary && (offset + mic->len) > boundary)
>> 
>> ^^ that should be enough for me to try to figure out what's doing wrong.
>> 
>> Ben
>> 
> 
> --
> Chuck Lever

--
Chuck Lever