Patchwork korina: fix probing crash

login
register
mail settings
Submitter Florian Fainelli
Date Jan. 11, 2009, 3:54 p.m.
Message ID <200901111654.24651.florian@openwrt.org>
Download mbox | patch
Permalink /patch/17817/
State Changes Requested
Delegated to: David Miller
Headers show

Comments

Florian Fainelli - Jan. 11, 2009, 3:54 p.m.
From: Florian Fainelli <florian@openwrt.org>
Subject: [PATCH] korina: fix probing crash

This patch fixes the wrong retrieval of platform_data
which makes the driver crash on probe. Propagate error
if the platform_driver cannot use its data.

Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
--
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
David Miller - Jan. 13, 2009, 5:52 a.m.
From: Florian Fainelli <florian@openwrt.org>
Date: Sun, 11 Jan 2009 16:54:24 +0100

> korina: fix probing crash
> 
> This patch fixes the wrong retrieval of platform_data
> which makes the driver crash on probe. Propagate error
> if the platform_driver cannot use its data.
> 
> Signed-off-by: Florian Fainelli <florian@openwrt.org>

There are several problems with this patch:

> @@ -1078,12 +1078,18 @@ static int korina_close(struct net_device *dev)
>  
>  static int korina_probe(struct platform_device *pdev)
>  {
> -	struct korina_device *bif = platform_get_drvdata(pdev);
> +	struct korina_device *bif;
>  	struct korina_private *lp;
>  	struct net_device *dev;
>  	struct resource *r;
>  	int rc;
>  
> +	bif = (struct korina_device *)pdev->dev.platform_data;

This cast is unnecessary, platform_data is "void *"

Secondly, korina_remove() also expects the korina_device pointer to be
in the platform drvdata.  It therefore has the same bug as
korina_probe() and needs to be fixed too.

But I suspect there is some other weird problem here.  If
you need to change from drvdata over to platform_data, then
this driver never could have been probed or removed properly.

This is absolutely a fundamental bug, where did it come from?
--
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
David Miller - Jan. 15, 2009, 5:47 a.m.
From: Phil Sutter <n0-1@freewrt.org>
Date: Wed, 14 Jan 2009 18:47:21 +0100

> Using platform_set_drvdata() here makes no sense, since the driver_data
> field has already been filled with valuable data (i.e. the MAC address).
> Also having driver_data point to the net_device is rather pointless
> since struct korina_device contains an apropriate field for it.
> 
> Signed-off-by: Phil Sutter <n0-1@freewrt.org>

Applied.
--
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

Patch

diff --git a/drivers/net/korina.c b/drivers/net/korina.c
index e185763..b2a3949 100644
--- a/drivers/net/korina.c
+++ b/drivers/net/korina.c
@@ -1078,12 +1078,18 @@  static int korina_close(struct net_device *dev)
 
 static int korina_probe(struct platform_device *pdev)
 {
-	struct korina_device *bif = platform_get_drvdata(pdev);
+	struct korina_device *bif;
 	struct korina_private *lp;
 	struct net_device *dev;
 	struct resource *r;
 	int rc;
 
+	bif = (struct korina_device *)pdev->dev.platform_data;
+	if (!bif) {
+		printk(KERN_ERR DRV_NAME ": missing platform data\n");
+		return -ENODEV;
+	}
+
 	dev = alloc_etherdev(sizeof(struct korina_private));
 	if (!dev) {
 		printk(KERN_ERR DRV_NAME ": alloc_etherdev failed\n");