netdev - Re: af_packet: use after free in prb_retire_rx_blk_timer

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:   Sat, 22 Jul 2017 22:59:08 -0700
From:   Cong Wang <xiyou.wangcong@...il.com>
To:     Ding Tianhong <dingtianhong@...wei.com>
Cc:     "liujian (CE)" <liujian56@...wei.com>,
        Willem de Bruijn <willemdebruijn.kernel@...il.com>,
        Dave Jones <davej@...emonkey.org.uk>,
        "alexander.levin@...izon.com" <alexander.levin@...izon.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "willemb@...gle.com" <willemb@...gle.com>,
        "daniel@...earbox.net" <daniel@...earbox.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: af_packet: use after free in prb_retire_rx_blk_timer_expired

On Sat, Jul 22, 2017 at 8:40 PM, Ding Tianhong <dingtianhong@...wei.com> wrote:
> Hi, Cong:
>
> Thanks for your quirk solution, but I still has some doubts about it,
> it looks like fix the problem in the packet_setsockopt->packet_set_ring processing,
> but when in packet_release processing, it may could not release the
> real pg_vec for the TPACKET_V3 ring, and then cause the mem leak,
> maybe I miss something here, nice to hear from your feedback. :)

Yes you miss that packet_release() has memset()'s so we won't hit
that path. :)

However, I missed the swap() in this messy function, actually I
believe the bug is that we modify tpacket_kbdq_core inside rx_ring
in non-closing case without actually stopping its timer. I feel
more confident with the following patch:


diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 008bb34ee324..267b181fef15 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -4263,6 +4263,7 @@ static int packet_set_ring(struct sock *sk,
union tpacket_req_u *req_u,
                case TPACKET_V3:
                        /* Block transmit is not supported yet */
                        if (!tx_ring) {
+                               prb_shutdown_retire_blk_timer(po, rb_queue);
                                init_prb_bdqc(po, rb, pg_vec, req_u);
                        } else {
                                struct tpacket_req3 *req3 = &req_u->req3;