linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Russell King <rmk+kernel@arm.linux.org.uk>
To: Florian Fainelli <f.fainelli@gmail.com>,
	David Miller <davem@davemloft.net>
Cc: devicetree@vger.kernel.org, Frank Rowand <frowand.list@gmail.com>,
	Grant Likely <grant.likely@linaro.org>,
	Iyappan Subramanian <isubramanian@apm.com>,
	Keyur Chudgar <kchudgar@apm.com>,
	linux-arm-kernel@lists.infradead.org,
	linuxppc-dev@lists.ozlabs.org, Li Yang <leoli@freescale.com>,
	Michal Simek <michal.simek@xilinx.com>,
	netdev@vger.kernel.org, Robert Richter <rric@kernel.org>,
	Rob Herring <robh+dt@kernel.org>,
	"Soren Brinkmann" <soren.brinkmann@xilinx.com>,
	Sunil Goutham <sgoutham@cavium.com>,
	Thomas Petazzoni <thomas.petazzoni@free-electrons.com>,
	linux-kernel@vger.kernel.org
Subject: [PATCH 2/9] net: dsa: fix of_mdio_find_bus() device refcount leak
Date: Tue, 22 Sep 2015 17:18:07 +0100	[thread overview]
Message-ID: <E1ZeQGd-0006XT-VN@rmk-PC.arm.linux.org.uk> (raw)
In-Reply-To: <20150922161710.GA21084@n2100.arm.linux.org.uk>

Current users of of_mdio_find_bus() leak a struct device refcount, as
they fail to clean up the reference obtained inside class_find_device().

Fix the DSA code to properly refcount the returned MDIO bus by:
1. taking a reference on the struct device whenever we assign it to
   pd->chip[x].host_dev.
2. dropping the reference when we overwrite the existing reference.
3. dropping the reference when we free the data structure.
4. dropping the initial reference we obtained after setting up the
   platform data structure, or on failure.

In step 2 above, where we obtain a new MDIO bus, there is no need to
take a reference on it as we would only have to drop it immediately
after assignment again, iow:

	put_device(cd->host_dev);	/* drop original assignment ref */
	cd->host_dev = get_device(&mdio_bus_switch->dev); /* get our ref */
	put_device(&mdio_bus_switch->dev); /* drop of_mdio_find_bus ref */

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 net/dsa/dsa.c | 38 +++++++++++++++++++++++++++++++-------
 1 file changed, 31 insertions(+), 7 deletions(-)

diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 76e3800765f8..bf4ba15fb780 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -634,6 +634,10 @@ static void dsa_of_free_platform_data(struct dsa_platform_data *pd)
 			port_index++;
 		}
 		kfree(pd->chip[i].rtable);
+
+		/* Drop our reference to the MDIO bus device */
+		if (pd->chip[i].host_dev)
+			put_device(pd->chip[i].host_dev);
 	}
 	kfree(pd->chip);
 }
@@ -661,16 +665,22 @@ static int dsa_of_probe(struct device *dev)
 		return -EPROBE_DEFER;
 
 	ethernet = of_parse_phandle(np, "dsa,ethernet", 0);
-	if (!ethernet)
-		return -EINVAL;
+	if (!ethernet) {
+		ret = -EINVAL;
+		goto out_put_mdio;
+	}
 
 	ethernet_dev = of_find_net_device_by_node(ethernet);
-	if (!ethernet_dev)
-		return -EPROBE_DEFER;
+	if (!ethernet_dev) {
+		ret = -EPROBE_DEFER;
+		goto out_put_mdio;
+	}
 
 	pd = kzalloc(sizeof(*pd), GFP_KERNEL);
-	if (!pd)
-		return -ENOMEM;
+	if (!pd) {
+		ret = -ENOMEM;
+		goto out_put_mdio;
+	}
 
 	dev->platform_data = pd;
 	pd->of_netdev = ethernet_dev;
@@ -691,7 +701,9 @@ static int dsa_of_probe(struct device *dev)
 		cd = &pd->chip[chip_index];
 
 		cd->of_node = child;
-		cd->host_dev = &mdio_bus->dev;
+
+		/* When assigning the host device, increment its refcount */
+		cd->host_dev = get_device(&mdio_bus->dev);
 
 		sw_addr = of_get_property(child, "reg", NULL);
 		if (!sw_addr)
@@ -711,6 +723,12 @@ static int dsa_of_probe(struct device *dev)
 				ret = -EPROBE_DEFER;
 				goto out_free_chip;
 			}
+
+			/* Drop the mdio_bus device ref, replacing the host
+			 * device with the mdio_bus_switch device, keeping
+			 * the refcount from of_mdio_find_bus() above.
+			 */
+			put_device(cd->host_dev);
 			cd->host_dev = &mdio_bus_switch->dev;
 		}
 
@@ -744,6 +762,10 @@ static int dsa_of_probe(struct device *dev)
 		}
 	}
 
+	/* The individual chips hold their own refcount on the mdio bus,
+	 * so drop ours */
+	put_device(&mdio_bus->dev);
+
 	return 0;
 
 out_free_chip:
@@ -751,6 +773,8 @@ static int dsa_of_probe(struct device *dev)
 out_free:
 	kfree(pd);
 	dev->platform_data = NULL;
+out_put_mdio:
+	put_device(&mdio_bus->dev);
 	return ret;
 }
 
-- 
2.1.0

  parent reply	other threads:[~2015-09-22 16:18 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-22 16:17 [PATCH v2 0/9] Phy, mdiobus, and netdev struct device fixes Russell King - ARM Linux
2015-09-22 16:18 ` [PATCH 1/9] phy: fix of_mdio_find_bus() device refcount leak Russell King
2015-09-22 16:18 ` Russell King [this message]
2015-09-22 16:18 ` [PATCH 3/9] phy: fix mdiobus module safety Russell King
2015-09-22 16:18 ` [PATCH 4/9] phy: add proper phy struct device refcounting Russell King
2015-09-22 16:18 ` [PATCH 5/9] of_mdio: fix MDIO phy " Russell King
2015-09-22 16:18 ` [PATCH 6/9] net: fix phy refcounting in a bunch of drivers Russell King
2015-09-22 16:18 ` [PATCH 7/9] phy: fixed-phy: properly validate phy in fixed_phy_update_state() Russell King
2015-09-22 16:18 ` [PATCH 8/9] phy: add phy_device_remove() Russell King
2015-09-22 16:18 ` [PATCH 9/9] net: fix net_device refcounting Russell King
2015-09-23 23:24 ` [PATCH v2 0/9] Phy, mdiobus, and netdev struct device fixes David Miller
2015-09-24 16:00   ` Russell King - ARM Linux
2015-09-24 19:35 ` [PATCH RESEND v3 1/9] phy: fix of_mdio_find_bus() device refcount leak Russell King
2015-09-24 19:35 ` [PATCH RESEND v3 2/9] net: dsa: " Russell King
2015-09-24 19:36 ` [PATCH RESEND v3 3/9] phy: fix mdiobus module safety Russell King
2015-09-24 19:36 ` [PATCH RESEND v3 4/9] phy: add proper phy struct device refcounting Russell King
2015-09-24 19:36 ` [PATCH RESEND v3 5/9] of_mdio: fix MDIO phy " Russell King
2015-09-24 22:21   ` Rob Herring
2015-09-24 19:36 ` [PATCH RESEND v3 6/9] net: fix phy refcounting in a bunch of drivers Russell King
2015-09-24 19:36 ` [PATCH RESEND v3 7/9] phy: fixed-phy: properly validate phy in fixed_phy_update_state() Russell King
2015-09-24 19:36 ` [PATCH RESEND v3 8/9] phy: add phy_device_remove() Russell King
2015-09-24 19:36 ` [PATCH RESEND v3 9/9] net: fix net_device refcounting Russell King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=E1ZeQGd-0006XT-VN@rmk-PC.arm.linux.org.uk \
    --to=rmk+kernel@arm.linux.org.uk \
    --cc=davem@davemloft.net \
    --cc=devicetree@vger.kernel.org \
    --cc=f.fainelli@gmail.com \
    --cc=frowand.list@gmail.com \
    --cc=grant.likely@linaro.org \
    --cc=isubramanian@apm.com \
    --cc=kchudgar@apm.com \
    --cc=leoli@freescale.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=michal.simek@xilinx.com \
    --cc=netdev@vger.kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=rric@kernel.org \
    --cc=sgoutham@cavium.com \
    --cc=soren.brinkmann@xilinx.com \
    --cc=thomas.petazzoni@free-electrons.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).