> On Nov 30, 2018, at 4:26 PM, Trond Myklebust <trondmy@xxxxxxxxx> wrote:
>
> On Fri, 2018-11-30 at 16:23 -0500, Chuck Lever wrote:
>>> On Nov 30, 2018, at 4:18 PM, Trond Myklebust <trondmy@xxxxxxxxx>
>>> wrote:
>>>
>>> A call to gss_wrap_req_priv() will end up replacing the original
>>> array
>>> in rqstp->rq_snd_buf.pages with a new one containing the encrypted
>>> data. In order to avoid having the rqstp->rq_snd_buf.bvec point to
>>> the
>>> wrong page data, we need to refresh that too.
>>>
>>> Signed-off-by: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
>>> ---
>>> net/sunrpc/xprtsock.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
>>> index ae77c71c1f64..615ef2397fc5 100644
>>> --- a/net/sunrpc/xprtsock.c
>>> +++ b/net/sunrpc/xprtsock.c
>>> @@ -843,6 +843,7 @@ static int xs_nospace(struct rpc_rqst *req)
>>> static void
>>> xs_stream_prepare_request(struct rpc_rqst *req)
>>> {
>>> + xdr_free_bvec(&req->rq_rcv_buf);
>>
>> I've added a pr_warn that fires whenever rq_rcv_buf.bvec != NULL,
>> and it never made a peep. Am I missing something?
>>
>
> It may be non-NULL,
I never found a case where call_encode was passing in a stale bvec array.
> but contain an array of pointers to the wrong
> pages. That's going to be the case when we re-encode the request,
> because (as your patch pointed out) we free the old array of
> rq_enc_pages (and its contents) and allocate a whole new array + set of
> pages.
I actually had a patch that looked exactly like yours (except for the
description text). It didn't seem to do anything. So there may be an
issue here, but I don't believe I'm seeing it with my reproducer.
I'll keep an eye out.
>>> req->rq_task->tk_status = xdr_alloc_bvec(&req->rq_rcv_buf,
>>> GFP_NOIO);
>>> }
>>>
>>> --
>>> 2.19.2
>>>
>>
>> --
>> Chuck Lever
--
Chuck Lever
