From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Gao Feng" Subject: RE: [PATCH net v3] driver: veth: Fix one possbile memleak when fail to register_netdevice Date: Wed, 3 May 2017 14:37:33 +0800 Message-ID: <006101d2c3d7$bf814230$3e83c690$@foxmail.com> References: <1493437911-27167-1-git-send-email-gfree.wind@foxmail.com> <000601d2c333$abb526d0$031f7470$@vip.163.com> <000c01d2c3b2$0925e880$1b71b980$@foxmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Cc: "'Gao Feng'" , "'davem'" , , "'Stephen Hemminger'" , , "'network dev'" To: "'Xin Long'" Return-path: Received: from smtpbg62.qq.com ([103.7.29.139]:16234 "EHLO smtpbg64.qq.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750966AbdECGhp (ORCPT ); Wed, 3 May 2017 02:37:45 -0400 In-Reply-To: Content-Language: zh-cn Sender: netdev-owner@vger.kernel.org List-ID: > From: Xin Long [mailto:lucien.xin@gmail.com] > Sent: Wednesday, May 3, 2017 1:38 PM > On Wed, May 3, 2017 at 10:07 AM, Gao Feng > wrote: > >> From: netdev-owner@vger.kernel.org > >> [mailto:netdev-owner@vger.kernel.org] > >> On Behalf Of Xin Long > >> Sent: Wednesday, May 3, 2017 12:59 AM On Tue, May 2, 2017 at 7:03 = PM, > >> Gao Feng wrote: > >> >> From: Xin Long [mailto:lucien.xin@gmail.com] > >> >> Sent: Tuesday, May 2, 2017 3:56 PM On Sat, Apr 29, 2017 at 11:51 > >> >> AM, wrote: > >> >> > From: Gao Feng [...] > > The fix you mentioned change the original logic. > > The dev->vstats is freed in advance in the ndo_uninit, not = destructor. > > It may break the backward. > Sorry, I didn't get your "backward" > I can't see there will be any problem caused by it. > can you say this patch also break the 'backward' ? > https://patchwork.ozlabs.org/patch/748964/ >=20 > It's really weird to do dev->reg_state check in ndo_unint ndo_unint is = supposed > to free the memory alloced in ndo_init. >=20 I am not sure if it would break the backward, so I said it MAY break. I assumed there may be someone would access the dev->vstats after = ndo_uninit, because current veth driver free the mem in the destructor. I selected this approach because I don't want to bring new bugs during = fix bug. If you're sure it is safe to free dev->vstats in ndo_uninit, I would = like to update it. BTW there are too many drivers which have possible memleak. You could find the list by = https://www.mail-archive.com/netdev@vger.kernel.org/msg166629.html. Some drivers allocate the resources in ndo_init, free some in ndo_uninit = and free left in destructor. I think there are some reasons.=20 We could not move all free in the ndo_uninit from destructor. What's = your opinion? Best Regards Feng