From mboxrd@z Thu Jan 1 00:00:00 1970 From: Cong Wang Subject: Re: af_packet: use after free in prb_retire_rx_blk_timer_expired Date: Sat, 22 Jul 2017 22:59:08 -0700 Message-ID: References: <20170410190350.ngfw435zzr7gpw7e@sasha-lappy> <20170410192309.35x7ddya2cyyv4y6@codemonkey.org.uk> <4F88C5DDA1E80143B232E89585ACE27D018C33F1@DGGEMA502-MBX.china.huawei.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Cc: "liujian (CE)" , Willem de Bruijn , Dave Jones , "alexander.levin@verizon.com" , "davem@davemloft.net" , "edumazet@google.com" , "willemb@google.com" , "daniel@iogearbox.net" , "netdev@vger.kernel.org" , "linux-kernel@vger.kernel.org" To: Ding Tianhong Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On Sat, Jul 22, 2017 at 8:40 PM, Ding Tianhong 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;