diff mbox

[1/1] starfire: clean up properly if firmware loading fails

Message ID 201001270427.o0R4R9EQ010839@imap1.linux-foundation.org
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Andrew Morton Jan. 27, 2010, 4:27 a.m. UTC
From: Ben Hutchings <bhutchings@solarflare.com>

netdev_open() will return without cleaning up net device or hardware state
if firmware loading fails.  This results in a BUG() on a second attempt to
bring the interface up, reported in
<http://bugzilla.kernel.org/show_bug.cgi?id=15117>, and probably has even
worse effects if the driver is removed afterwards.

Call netdev_close() to clean up on failure.

Addresses http://bugzilla.kernel.org/show_bug.cgi?id=15091

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
Reported-by: Michael Moffatt <michael@moffatt.org.nz>
Tested-by: Michael Moffatt <michael@moffatt.org.nz>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: <stable@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/net/starfire.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

David Miller Jan. 27, 2010, 4:49 a.m. UTC | #1
From: akpm@linux-foundation.org
Date: Tue, 26 Jan 2010 20:27:09 -0800

> From: Ben Hutchings <bhutchings@solarflare.com>
> 
> netdev_open() will return without cleaning up net device or hardware state
> if firmware loading fails.  This results in a BUG() on a second attempt to
> bring the interface up, reported in
> <http://bugzilla.kernel.org/show_bug.cgi?id=15117>, and probably has even
> worse effects if the driver is removed afterwards.
> 
> Call netdev_close() to clean up on failure.
> 
> Addresses http://bugzilla.kernel.org/show_bug.cgi?id=15091
> 
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> Reported-by: Michael Moffatt <michael@moffatt.org.nz>
> Tested-by: Michael Moffatt <michael@moffatt.org.nz>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: <stable@kernel.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

Applied, thanks everyone!
--
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
Ben Hutchings Jan. 27, 2010, 4:58 a.m. UTC | #2
On Tue, 2010-01-26 at 20:27 -0800, akpm@linux-foundation.org wrote:
> From: Ben Hutchings <bhutchings@solarflare.com>
> 
> netdev_open() will return without cleaning up net device or hardware state
> if firmware loading fails.  This results in a BUG() on a second attempt to
> bring the interface up, reported in
> <http://bugzilla.kernel.org/show_bug.cgi?id=15117>, and probably has even
> worse effects if the driver is removed afterwards.

David, please correct the URL above before applying the patch so I don't
continue to look like an idiot. :-)  It should be
<http://bugzilla.kernel.org/show_bug.cgi?id=15091> as added by Andrew
below.

Ben.
David Miller Jan. 27, 2010, 5:05 a.m. UTC | #3
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Wed, 27 Jan 2010 04:58:17 +0000

> On Tue, 2010-01-26 at 20:27 -0800, akpm@linux-foundation.org wrote:
>> From: Ben Hutchings <bhutchings@solarflare.com>
>> 
>> netdev_open() will return without cleaning up net device or hardware state
>> if firmware loading fails.  This results in a BUG() on a second attempt to
>> bring the interface up, reported in
>> <http://bugzilla.kernel.org/show_bug.cgi?id=15117>, and probably has even
>> worse effects if the driver is removed afterwards.
> 
> David, please correct the URL above before applying the patch so I don't
> continue to look like an idiot. :-)  It should be
> <http://bugzilla.kernel.org/show_bug.cgi?id=15091> as added by Andrew
> below.

Done.
--
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 mbox

Patch

diff -puN drivers/net/starfire.c~starfire-clean-up-properly-if-firmware-loading-fails drivers/net/starfire.c
--- a/drivers/net/starfire.c~starfire-clean-up-properly-if-firmware-loading-fails
+++ a/drivers/net/starfire.c
@@ -1063,7 +1063,7 @@  static int netdev_open(struct net_device
 	if (retval) {
 		printk(KERN_ERR "starfire: Failed to load firmware \"%s\"\n",
 		       FIRMWARE_RX);
-		return retval;
+		goto out_init;
 	}
 	if (fw_rx->size % 4) {
 		printk(KERN_ERR "starfire: bogus length %zu in \"%s\"\n",
@@ -1108,6 +1108,9 @@  out_tx:
 	release_firmware(fw_tx);
 out_rx:
 	release_firmware(fw_rx);
+out_init:
+	if (retval)
+		netdev_close(dev);
 	return retval;
 }