Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753862AbdGXKkX (ORCPT ); Mon, 24 Jul 2017 06:40:23 -0400 Received: from szxga03-in.huawei.com ([45.249.212.189]:9408 "EHLO szxga03-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752799AbdGXKkM (ORCPT ); Mon, 24 Jul 2017 06:40:12 -0400 From: "liujian (CE)" To: Dingtianhong , Cong Wang CC: 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" Subject: RE: af_packet: use after free in prb_retire_rx_blk_timer_expired Thread-Topic: af_packet: use after free in prb_retire_rx_blk_timer_expired Thread-Index: AQHSsi0kPKd7RxFbFUioT9017kTPL6G+dU+AgAHVPYCAn++J8IAAFXSAgACQ+ICAACagAIAArApggAAUuoCAAC0bcP//y8MAgACH6ACAAAVAgIABEmRA Date: Mon, 24 Jul 2017 10:29:59 +0000 Message-ID: <4F88C5DDA1E80143B232E89585ACE27D018CDC49@DGGEMA502-MBS.china.huawei.com> References: <20170410190350.ngfw435zzr7gpw7e@sasha-lappy> <20170410192309.35x7ddya2cyyv4y6@codemonkey.org.uk> <4F88C5DDA1E80143B232E89585ACE27D018C33F1@DGGEMA502-MBX.china.huawei.com> <4F88C5DDA1E80143B232E89585ACE27D018CB6E0@DGGEMA502-MBS.china.huawei.com> <8f6c2fec-0ac4-1414-69da-462114a4481f@huawei.com> In-Reply-To: Accept-Language: en-US Content-Language: zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.177.97.126] Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A020204.5975CC31.0074,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=169.254.3.238, so=2014-11-16 11:51:01, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 55c54cd349523a5ebacf567c661ac09e Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by nfs id v6OAeSGi028750 Content-Length: 2660 Lines: 90 Hi Wang cong, After apply the patch, I did not hit the issue again. Thank you~ Best Regards, liujian > -----Original Message----- > From: Dingtianhong > Sent: Monday, July 24, 2017 9:29 AM > To: Cong Wang; liujian (CE) > Cc: 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 > Subject: Re: af_packet: use after free in prb_retire_rx_blk_timer_expired > > > > On 2017/7/24 9:09, Ding Tianhong wrote: > > > > > > On 2017/7/24 1:03, Cong Wang wrote: > >> On Sun, Jul 23, 2017 at 5:48 AM, liujian (CE) wrote: > >>> Hi > >>> > >>> I find it caused by below steps: > >>> 1. set tp_version to TPACKET_V3 and req->tp_block_nr to 1 2. set > >>> tp_block_nr to 0 Then pg_vec was freed, and we did not delete the > >>> timer? > >> > >> Thanks for testing! > >> > >> Ah, I overlook the initialization case in my previous patch. > >> > >> How about the following one? Does it cover all the cases? > >> > >> > >> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index > >> 008bb34ee324..0615c2a950fa 100644 > >> --- a/net/packet/af_packet.c > >> +++ b/net/packet/af_packet.c > >> @@ -4329,7 +4329,7 @@ static int packet_set_ring(struct sock *sk, > >> union tpacket_req_u *req_u, > >> register_prot_hook(sk); > >> } > >> spin_unlock(&po->bind_lock); > >> - if (closing && (po->tp_version > TPACKET_V2)) { > >> + if (pg_vec && (po->tp_version > TPACKET_V2)) { > >> /* Because we don't support block-based V3 on tx-ring > */ > >> if (!tx_ring) > >> prb_shutdown_retire_blk_timer(po, > rb_queue); > >> > >> . > > > > Hi, Cong: > > > > It looks like could not cover the case: req->tp_block_nr = 2 -> > reg->tp_block_nr = 1 . > > > > Oh, looks like this case would never happen, so I think your solution is ok. > > > what about this way: > > --- a/net/packet/af_packet.c > > +++ b/net/packet/af_packet.c > > @@ -4331,13 +4331,17 @@ static int packet_set_ring(struct sock *sk, union > tpacket_req_u *req_u, > > register_prot_hook(sk); > > } > > spin_unlock(&po->bind_lock); > > - if (closing && (po->tp_version > TPACKET_V2)) { > > + if ((closing || (pg_vec && !reg->tp_block_nr))&& > > + (po->tp_version > TPACKET_V2)) { > > /* Because we don't support block-based V3 on tx-ring */ > > if (!tx_ring) > > prb_shutdown_retire_blk_timer(po, > rb_queue); > > > > > > >> > > > > > > . > >