From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4A626C433EF for ; Tue, 28 Sep 2021 13:11:18 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 2528261131 for ; Tue, 28 Sep 2021 13:11:18 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S240648AbhI1NMy (ORCPT ); Tue, 28 Sep 2021 09:12:54 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33530 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235776AbhI1NMx (ORCPT ); Tue, 28 Sep 2021 09:12:53 -0400 Received: from pandora.armlinux.org.uk (pandora.armlinux.org.uk [IPv6:2001:4d48:ad52:32c8:5054:ff:fe00:142]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EDC42C061575; Tue, 28 Sep 2021 06:11:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=armlinux.org.uk; s=pandora-2019; h=Sender:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=8Y4+qGyuCO6q39+CSQ6y4fo3O7zf87El/V8Z1fXLg+o=; b=hZMyLWbpDXk1dFAoVgKVJW6UM5 /7QaKqfb6x7Q5wHPPutTiLBI++SDHPaSdJDbsA9SucV/fdgGEVXZ5ClufUPGbJJl0hew3o0IKHwD4 Wi+HhA3ERsYP9a+6lPvW9rbaat4K3hJC8VLOXHq+uXY0LaBYNTpnsYCkxFf3M3b/CsXuJp4hnVbTz 7mN/w1syAaQM5nYuvw/r/5h3UW+aYG20kS+TKp1RhW1kRlrNo5n5W2rARuO1WQqgz7tAm+ixkR2F0 SoDGH2rRokPB9PyXRTOWIve0oGQeRs+IBgF/RKDwhOzLQIlx3kAc1lYDQeO7TQIUspuwYWouO+XvG nzh55p7g==; Received: from shell.armlinux.org.uk ([fd8f:7570:feb6:1:5054:ff:fe00:4ec]:54830) by pandora.armlinux.org.uk with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1mVCsz-000191-Pc; Tue, 28 Sep 2021 14:11:05 +0100 Received: from linux by shell.armlinux.org.uk with local (Exim 4.94.2) (envelope-from ) id 1mVCsy-0001ma-1p; Tue, 28 Sep 2021 14:11:04 +0100 Date: Tue, 28 Sep 2021 14:11:04 +0100 From: "Russell King (Oracle)" To: Yanfei Xu Cc: andrew@lunn.ch, hkallweit1@gmail.com, davem@davemloft.net, kuba@kernel.org, p.zabel@pengutronix.de, syzbot+398e7dc692ddbbb4cfec@syzkaller.appspotmail.com, linux-kernel@vger.kernel.org, netdev@vger.kernel.org Subject: Re: [PATCH] net: mdiobus: Fix memory leak in __mdiobus_register Message-ID: References: <20210926045313.2267655-1-yanfei.xu@windriver.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210926045313.2267655-1-yanfei.xu@windriver.com> Sender: Russell King (Oracle) Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, Sep 26, 2021 at 12:53:13PM +0800, Yanfei Xu wrote: > Once device_register() failed, we should call put_device() to > decrement reference count for cleanup. Or it will cause memory > leak. > > BUG: memory leak > unreferenced object 0xffff888114032e00 (size 256): > comm "kworker/1:3", pid 2960, jiffies 4294943572 (age 15.920s) > hex dump (first 32 bytes): > 00 00 00 00 00 00 00 00 08 2e 03 14 81 88 ff ff ................ > 08 2e 03 14 81 88 ff ff 90 76 65 82 ff ff ff ff .........ve..... > backtrace: > [] kmalloc include/linux/slab.h:591 [inline] > [] kzalloc include/linux/slab.h:721 [inline] > [] device_private_init drivers/base/core.c:3203 [inline] > [] device_add+0x89b/0xdf0 drivers/base/core.c:3253 > [] __mdiobus_register+0xc3/0x450 drivers/net/phy/mdio_bus.c:537 > [] __devm_mdiobus_register+0x75/0xf0 drivers/net/phy/mdio_devres.c:87 > [] ax88772_init_mdio drivers/net/usb/asix_devices.c:676 [inline] > [] ax88772_bind+0x330/0x480 drivers/net/usb/asix_devices.c:786 > [] usbnet_probe+0x3ff/0xdf0 drivers/net/usb/usbnet.c:1745 > [] usb_probe_interface+0x177/0x370 drivers/usb/core/driver.c:396 > [] call_driver_probe drivers/base/dd.c:517 [inline] > [] really_probe.part.0+0xe7/0x380 drivers/base/dd.c:596 > [] really_probe drivers/base/dd.c:558 [inline] > [] __driver_probe_device+0x10c/0x1e0 drivers/base/dd.c:751 > [] driver_probe_device+0x2a/0x120 drivers/base/dd.c:781 > [] __device_attach_driver+0xf6/0x140 drivers/base/dd.c:898 > [] bus_for_each_drv+0xb7/0x100 drivers/base/bus.c:427 > [] __device_attach+0x122/0x260 drivers/base/dd.c:969 > [] bus_probe_device+0xc6/0xe0 drivers/base/bus.c:487 > [] device_add+0x5fb/0xdf0 drivers/base/core.c:3359 > [] usb_set_configuration+0x9d9/0xb90 drivers/usb/core/message.c:2170 > [] usb_generic_driver_probe+0x8c/0xc0 drivers/usb/core/generic.c:238 > > BUG: memory leak > unreferenced object 0xffff888116f06900 (size 32): > comm "kworker/0:2", pid 2670, jiffies 4294944448 (age 7.160s) > hex dump (first 32 bytes): > 75 73 62 2d 30 30 31 3a 30 30 33 00 00 00 00 00 usb-001:003..... > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ > backtrace: > [] kstrdup+0x36/0x70 mm/util.c:60 > [] kstrdup_const+0x53/0x80 mm/util.c:83 > [] kvasprintf_const+0xc2/0x110 lib/kasprintf.c:48 > [] kobject_set_name_vargs+0x3b/0xe0 lib/kobject.c:289 > [] dev_set_name+0x63/0x90 drivers/base/core.c:3147 > [] __mdiobus_register+0xbb/0x450 drivers/net/phy/mdio_bus.c:535 > [] __devm_mdiobus_register+0x75/0xf0 drivers/net/phy/mdio_devres.c:87 > [] ax88772_init_mdio drivers/net/usb/asix_devices.c:676 [inline] > [] ax88772_bind+0x330/0x480 drivers/net/usb/asix_devices.c:786 > [] usbnet_probe+0x3ff/0xdf0 drivers/net/usb/usbnet.c:1745 > [] usb_probe_interface+0x177/0x370 drivers/usb/core/driver.c:396 > [] call_driver_probe drivers/base/dd.c:517 [inline] > [] really_probe.part.0+0xe7/0x380 drivers/base/dd.c:596 > [] really_probe drivers/base/dd.c:558 [inline] > [] __driver_probe_device+0x10c/0x1e0 drivers/base/dd.c:751 > [] driver_probe_device+0x2a/0x120 drivers/base/dd.c:781 > [] __device_attach_driver+0xf6/0x140 drivers/base/dd.c:898 > [] bus_for_each_drv+0xb7/0x100 drivers/base/bus.c:427 > [] __device_attach+0x122/0x260 drivers/base/dd.c:969 > > Reported-by: syzbot+398e7dc692ddbbb4cfec@syzkaller.appspotmail.com > Signed-off-by: Yanfei Xu > --- > drivers/net/phy/mdio_bus.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c > index 53f034fc2ef7..6f4b4e5df639 100644 > --- a/drivers/net/phy/mdio_bus.c > +++ b/drivers/net/phy/mdio_bus.c > @@ -537,6 +537,7 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner) > err = device_register(&bus->dev); > if (err) { > pr_err("mii_bus %s failed to register\n", bus->id); > + put_device(&bus->dev); > return -EINVAL; > } This patch is incorrect: 1) the reported failure does not involve this path. 2) device_register() failing does not need a put_device() because the contained device_add() undoes everything that it attempted to do. The above backtraces occur because we have had a successful device_register() fall, but later call device_del() and then kfree() the mdiobus, which has an embedded the struct device that has pointers to memory that has not been cleaned up - because kfree() is the wrong way to handle this. bus->state needs to be set to indicate that the embedded struct device has been registered but no longer is registered if we fail after device_register() has been called. If device_register() fails, then there is no problem. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!