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]

Date:   Fri, 6 Jul 2018 13:46:34 +0900
From:   Lorenzo Colitti <lorenzo@...gle.com>
To:     Subash Abhinov Kasiviswanathan <subashab@...eaurora.org>
Cc:     netdev@...r.kernel.org, Eric Dumazet <eric.dumazet@...il.com>,
        Alistair Strachan <astrachan@...gle.com>
Subject: Re: Crash due to destroying TCP request sockets using SOCK_DESTROY

On Fri, Jul 6, 2018 at 11:37 AM Subash Abhinov Kasiviswanathan
<subashab@...eaurora.org> 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 =
> 0xffffff801cf98000)
>   13232.479998:   <2> Call trace:
>   13232.480000:   <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?