netdev - Re: Crash due to destroying TCP request sockets using SOCK

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2e8c55a7adff3aff6e62738c793e84e3@codeaurora.org>
Date:   Fri, 06 Jul 2018 17:24:04 -0600
From:   Subash Abhinov Kasiviswanathan <subashab@...eaurora.org>
To:     Eric Dumazet <eric.dumazet@...il.com>,
        Lorenzo Colitti <lorenzo@...gle.com>
Cc:     netdev@...r.kernel.org, Alistair Strachan <astrachan@...gle.com>,
        David Ahern <dsa@...ulusnetworks.com>
Subject: Re: Crash due to destroying TCP request sockets using SOCK_DESTROY

>> Looks like for a TCP_NEW_SYN_RECV socket, sock_diag_destroy
>> essentially ends up doing:
>> 
>>                         struct request_sock *req = inet_reqsk(sk);
>> 
>>                         local_bh_disable();
>>                         
>> inet_csk_reqsk_queue_drop_and_put(req->rsk_listener,
>>                                                           req);
>>                         local_bh_enable();
>> ...
>> 
>>         sock_gen_put(sk);
>> 
>> It looks like inet_csk_reqsk_queue_drop_and_put calls reqsk_put(req),
>> which frees the socket, and at that point sock_gen_put is a UAF. Do we
>> just need:
>> 
>> -                        
>> inet_csk_reqsk_queue_drop_and_put(req->rsk_listener,
>> -                                                           req);
>> +                        inet_csk_reqsk_queue_drop(req->rsk_listener, 
>> req);
>> 
>> since sock_gen_put will also end up calling reqsk_put() for a
>> TCP_SYN_RECV socket?
>> 
>> Alastair - you're able to reproduce this UAF using net_test on qemu,
>> right? If so, could you try that two-line patch above?
>> 
> 
> Hi Lorenzo
> 
> Your patch makes sense to me, please submit it formally with :
> 
> Fixes: d7226c7a4dd1 ("net: diag: Fix refcnt leak in error path
> destroying socket")
> Cc: David Ahern <dsa@...ulusnetworks.com>
> 
> Thanks !

Thanks Lorenzo and Eric. I will try it out locally.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project