Re: Crash due to destroying TCP request sockets using SOCK_DESTROY
On Fri, Jul 6, 2018 at 7:24 PM Eric Dumazet wrote:
>
> 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
Submitted a patch against net: https://patchwork.ozlabs.org/patch/940757/ .
Cheers,
Lorenzo
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
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
Re: Crash due to destroying TCP request sockets using SOCK_DESTROY
On 07/05/2018 09:46 PM, Lorenzo Colitti wrote:
> On Fri, Jul 6, 2018 at 11:37 AM Subash Abhinov Kasiviswanathan
> wrote:
>>
>> From the call stack, a TCP socket is being destroyed using netlink_diag.
>> The memory dump showed that the socket was an inet request socket (in
>> state TCP_NEW_SYN_RECV) with refcount of 0.
>> [...]
>> 13232.479820: <2> refcount_t: underflow; use-after-free.
>> 13232.479838: <6> [ cut here ]
>> 13232.479843: <6> kernel BUG at kernel/msm-4.14/lib/refcount.c:204!
>> 13232.479849: <6> Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
>> [...]
>> 13232.479996: <6> Process netd (pid: 648, stack limit =
>> 0xff801cf98000)
>> 13232.479998: <2> Call trace:
>> 13232.48: <2> refcount_sub_and_test+0x64/0x78
>> 13232.480002: <2> refcount_dec_and_test+0x18/0x24
>> 13232.480005: <2> sock_gen_put+0x1c/0xb0
>> 13232.480009: <2> tcp_diag_destroy+0x54/0x68
>> [...]
>
> 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
Thanks !
Re: Crash due to destroying TCP request sockets using SOCK_DESTROY
On Fri, Jul 6, 2018 at 11:37 AM Subash Abhinov Kasiviswanathan wrote: > > From the call stack, a TCP socket is being destroyed using netlink_diag. > The memory dump showed that the socket was an inet request socket (in > state TCP_NEW_SYN_RECV) with refcount of 0. > [...] > 13232.479820: <2> refcount_t: underflow; use-after-free. > 13232.479838: <6> [ cut here ] > 13232.479843: <6> kernel BUG at kernel/msm-4.14/lib/refcount.c:204! > 13232.479849: <6> Internal error: Oops - BUG: 0 [#1] PREEMPT SMP > [...] > 13232.479996: <6> Process netd (pid: 648, stack limit = > 0xff801cf98000) > 13232.479998: <2> Call trace: > 13232.48: <2> refcount_sub_and_test+0x64/0x78 > 13232.480002: <2> refcount_dec_and_test+0x18/0x24 > 13232.480005: <2> sock_gen_put+0x1c/0xb0 > 13232.480009: <2> tcp_diag_destroy+0x54/0x68 > [...] 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?
Crash due to destroying TCP request sockets using SOCK_DESTROY
We are seeing a crash on an ARM64 device with Android 4.14 based kernel. From the call stack, a TCP socket is being destroyed using netlink_diag. The memory dump showed that the socket was an inet request socket (in state TCP_NEW_SYN_RECV) with refcount of 0. The crash seems to have happened during a regression test where wifi was toggled with some browser activity but it is not very easily reproducible. I believe netd on Android tries to destroy all sockets in a system on change of network. 13232.479820: <2> refcount_t: underflow; use-after-free. 13232.479838: <6> [ cut here ] 13232.479843: <6> kernel BUG at kernel/msm-4.14/lib/refcount.c:204! 13232.479849: <6> Internal error: Oops - BUG: 0 [#1] PREEMPT SMP 13232.479895: <6> CPU: 4 PID: 648 Comm: netd Tainted: G S W O 4.14.49+ #1 13232.479897: <6> task: fff5d6e28080 task.stack: ff801cf98000 13232.479908: <2> pc : refcount_sub_and_test+0x64/0x78 13232.479910: <2> lr : refcount_sub_and_test+0x64/0x78 13232.479911: <2> sp : ff801cf9ba40 pstate : 20400145 13232.479911: <2> x29: ff801cf9ba40 x28: fff5d6e28080 13232.479914: <2> x27: ff801cf9bd10 x26: fff4a1428f40 13232.479915: <2> x25: x24: ff91 13232.479917: <2> x23: 0015 x22: fff5b837c880 13232.479919: <2> x21: fff4a1428f40 x20: 13232.479920: <2> x19: fff4c47c6088 x18: e7b13cd1ecbfea00 13232.479922: <2> x17: 0008ec3bb553 x16: 011d8776aa792786 13232.479924: <2> x15: e7b13cd1ecbfea00 x14: 2bdb7692 13232.479925: <2> x13: x12: e7b13cd1ecbfea00 13232.479927: <2> x11: e7b13cd1ecbfea00 x10: 13232.479928: <2> x9 : e7b13cd1ecbfea00 x8 : 13232.479929: <2> x7 : 0001 x6 : 0001 13232.479931: <2> x5 : x4 : 0c08ed425d69 13232.479932: <2> x3 : 0066effb6000 x2 : ff8f09dc5000 13232.479934: <2> x1 : x0 : 0026 13232.479996: <6> Process netd (pid: 648, stack limit = 0xff801cf98000) 13232.479998: <2> Call trace: 13232.48: <2> refcount_sub_and_test+0x64/0x78 13232.480002: <2> refcount_dec_and_test+0x18/0x24 13232.480005: <2> sock_gen_put+0x1c/0xb0 13232.480009: <2> tcp_diag_destroy+0x54/0x68 13232.480010: <2> inet_diag_cmd_exact+0x78/0xa0 13232.480012: <2> inet_diag_handler_cmd+0xcc/0xf8 13232.480018: <2> sock_diag_rcv_msg+0x130/0x158 13232.480021: <2> netlink_rcv_skb+0xa4/0x11c 13232.480023: <2> sock_diag_rcv+0x34/0x48 13232.480025: <2> netlink_unicast+0x158/0x1f0 13232.480026: <2> netlink_sendmsg+0x334/0x340 13232.480028: <2> sock_sendmsg+0x44/0x60 13232.480031: <2> sock_write_iter+0xac/0xf4 13232.480034: <2> __vfs_write+0x124/0x154 13232.480036: <2> vfs_write+0xcc/0x188 13232.480038: <2> SyS_write+0x60/0xc0 13232.480040: <2> el0_svc_naked+0x34/0x38 13232.480042: <6> Code: 910003fd f0008200 910fd000 97f4158c (d421) 13232.480045: <6> ---[ end trace 994bad5b8077e394 ]--- 13232.480061: <6> Kernel panic - not syncing: Fatal exception -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
