Re: [PATCH 1/4] SUNRPC: Don't leak netobj memory when gss_read_proxy_verf() fails

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> On Nov 28, 2022, at 9:13 AM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> 
> On Mon, 2022-11-28 at 14:02 +0000, Chuck Lever III wrote:
>> 
>>> On Nov 28, 2022, at 8:11 AM, Jeff Layton <jlayton@xxxxxxxxxxxxxxx> wrote:
>>> 
>>> On Sat, 2022-11-26 at 15:55 -0500, Chuck Lever wrote:
>>>> Fixes: 030d794bf498 ("SUNRPC: Use gssproxy upcall for server RPCGSS authentication.")
>>>> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
>>>> Cc: <stable@xxxxxxxxxxxxxxx>
>>>> ---
>>>> net/sunrpc/auth_gss/svcauth_gss.c |    9 +++++++--
>>>> 1 file changed, 7 insertions(+), 2 deletions(-)
>>>> 
>>>> diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
>>>> index bcd74dddbe2d..9a5db285d4ae 100644
>>>> --- a/net/sunrpc/auth_gss/svcauth_gss.c
>>>> +++ b/net/sunrpc/auth_gss/svcauth_gss.c
>>>> @@ -1162,18 +1162,23 @@ static int gss_read_proxy_verf(struct svc_rqst *rqstp,
>>>> 		return res;
>>>> 
>>>> 	inlen = svc_getnl(argv);
>>>> -	if (inlen > (argv->iov_len + rqstp->rq_arg.page_len))
>>>> +	if (inlen > (argv->iov_len + rqstp->rq_arg.page_len)) {
>>>> +		kfree(in_handle->data);
>>> 
>>> I wish this were more obvious in this code. It's not at all evident to
>>> the casual reader that gss_read_common_verf calls dup_netobj here and
>>> that you need to clean up after it. At a bare minimum, we ought to have
>>> a comment to that effect over gss_read_common_verf. While you're in
>>> there, that function is also pretty big to be marked static inline. Can
>>> you change that too? Ditto for gss_read_verf.
>> 
>> Agreed: I've done that clean up in subsequent patches that are part
>> of the (yet to be posted) series to replace svc_get/putnl with
>> xdr_stream.
>> 
>> This seemed like a good fix to apply earlier rather than later. That
>> should enable it to be backported cleanly.
>> 
> 
> 
> Fair enough. You can add my Reviewed-by to the whole series.

Thanks!


> I'll look forward to seeing the full cleanup.

It's several dozen patches, and it's currently based on something
close to what's going into v6.2. So my plan is to rebase it on
v6.2-rc1 when that becomes available and then post the first half
of it (the decode part) at that time.


--
Chuck Lever







[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux