From patchwork Sun Nov 9 04:34:47 2008 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lennert Buytenhek X-Patchwork-Id: 7902 X-Patchwork-Delegate: jgarzik@pobox.com Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by ozlabs.org (Postfix) with ESMTP id 9C82DDDE03 for ; Sun, 9 Nov 2008 15:34:55 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753919AbYKIEeu (ORCPT ); Sat, 8 Nov 2008 23:34:50 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1753907AbYKIEeu (ORCPT ); Sat, 8 Nov 2008 23:34:50 -0500 Received: from xi.wantstofly.org ([80.101.37.227]:52672 "EHLO xi.wantstofly.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753899AbYKIEeu (ORCPT ); Sat, 8 Nov 2008 23:34:50 -0500 Received: by xi.wantstofly.org (Postfix, from userid 500) id DD4D27F9D0; Sun, 9 Nov 2008 05:34:47 +0100 (CET) Date: Sun, 9 Nov 2008 05:34:47 +0100 From: Lennert Buytenhek To: jeff@garzik.org, Bryan Wu Cc: netdev@vger.kernel.org Subject: Re: [PATCH] phylib: fix premature freeing of struct mii_bus Message-ID: <20081109043447.GB17630@xi.wantstofly.org> References: <20081107210617.GS32552@xi.wantstofly.org> <20081109015950.GX32552@xi.wantstofly.org> Mime-Version: 1.0 Content-Disposition: inline In-Reply-To: <20081109015950.GX32552@xi.wantstofly.org> User-Agent: Mutt/1.4.2.2i Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Sun, Nov 09, 2008 at 02:59:50AM +0100, Lennert Buytenhek wrote: > > From: Lennert Buytenhek > > > > Commit 46abc02175b3c246dd5141d878f565a8725060c9 ("phylib: give mdio > > buses a device tree presence") added a call to device_unregister() in > > a situation where the caller did not expect the reference count on > > the embedded kobject to be dropped, but device_unregister() does drop > > that reference count. The right function to use in this situation > > is device_del(), which unregisters the device from the system like > > device_unregister() does, but without dropping the reference count. > > Wait a minute -- the original code _is_ correct. mdiobus_register() > bumps the refcount to two (once because of device_initialize(), once > because of device_add()), mdiobus_unregister() drops it back to one, > and mdiobus_free() drops it to zero. I was thrown off by a different bug here. The original code is not correct after all, and mdiobus_unregister() does have a bug where it drops the refcount by one too many. So the patch in the original mail was correct, but the commit message was not. Can you please use this message+patch instead? commit 19e92973fc187125af02cefabc043359a962e5be Author: Lennert Buytenhek Date: Sun Nov 9 05:02:11 2008 +0100 phylib: fix premature freeing of struct mii_bus Commit 46abc02175b3c246dd5141d878f565a8725060c9 ("phylib: give mdio buses a device tree presence") added a call to device_unregister() in a situation where the caller did not intend for the device to be freed yet, but apart from just unregistering the device from the system, device_unregister() does an additional put_device() that is intended to free it. The right function to use in this situation is device_del(), which unregisters the device from the system like device_unregister() does, but without dropping the reference count an additional time. Bug report from Bryan Wu . Signed-off-by: Lennert Buytenhek Tested-by: Bryan Wu --- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c index 6671e2d..df06301 100644 --- a/drivers/net/phy/mdio_bus.c +++ b/drivers/net/phy/mdio_bus.c @@ -135,7 +135,7 @@ void mdiobus_unregister(struct mii_bus *bus) BUG_ON(bus->state != MDIOBUS_REGISTERED); bus->state = MDIOBUS_UNREGISTERED; - device_unregister(&bus->dev); + device_del(&bus->dev); for (i = 0; i < PHY_MAX_ADDR; i++) { if (bus->phy_map[i]) device_unregister(&bus->phy_map[i]->dev);