Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751700AbdGWF7c (ORCPT ); Sun, 23 Jul 2017 01:59:32 -0400 Received: from mail-wm0-f49.google.com ([74.125.82.49]:35992 "EHLO mail-wm0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750825AbdGWF7a (ORCPT ); Sun, 23 Jul 2017 01:59:30 -0400 MIME-Version: 1.0 In-Reply-To: References: <20170410190350.ngfw435zzr7gpw7e@sasha-lappy> <20170410192309.35x7ddya2cyyv4y6@codemonkey.org.uk> <4F88C5DDA1E80143B232E89585ACE27D018C33F1@DGGEMA502-MBX.china.huawei.com> From: Cong Wang Date: Sat, 22 Jul 2017 22:59:08 -0700 Message-ID: Subject: Re: af_packet: use after free in prb_retire_rx_blk_timer_expired To: Ding Tianhong 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" Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1423 Lines: 31 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;