netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [patch 1/1] starfire: clean up properly if firmware loading fails
@ 2010-01-27  4:27 akpm
  2010-01-27  4:49 ` David Miller
  2010-01-27  4:58 ` Ben Hutchings
  0 siblings, 2 replies; 4+ messages in thread
From: akpm @ 2010-01-27  4:27 UTC (permalink / raw)
  To: davem; +Cc: netdev, akpm, bhutchings, michael, stable

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(-)

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;
 }
 
_

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [patch 1/1] starfire: clean up properly if firmware loading fails
  2010-01-27  4:27 [patch 1/1] starfire: clean up properly if firmware loading fails akpm
@ 2010-01-27  4:49 ` David Miller
  2010-01-27  4:58 ` Ben Hutchings
  1 sibling, 0 replies; 4+ messages in thread
From: David Miller @ 2010-01-27  4:49 UTC (permalink / raw)
  To: akpm; +Cc: netdev, bhutchings, michael, stable

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!

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [patch 1/1] starfire: clean up properly if firmware loading fails
  2010-01-27  4:27 [patch 1/1] starfire: clean up properly if firmware loading fails akpm
  2010-01-27  4:49 ` David Miller
@ 2010-01-27  4:58 ` Ben Hutchings
  2010-01-27  5:05   ` David Miller
  1 sibling, 1 reply; 4+ messages in thread
From: Ben Hutchings @ 2010-01-27  4:58 UTC (permalink / raw)
  To: davem; +Cc: akpm, netdev, michael, stable

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.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [patch 1/1] starfire: clean up properly if firmware loading fails
  2010-01-27  4:58 ` Ben Hutchings
@ 2010-01-27  5:05   ` David Miller
  0 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2010-01-27  5:05 UTC (permalink / raw)
  To: bhutchings; +Cc: akpm, netdev, michael, stable

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.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2010-01-27  5:05 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-01-27  4:27 [patch 1/1] starfire: clean up properly if firmware loading fails akpm
2010-01-27  4:49 ` David Miller
2010-01-27  4:58 ` Ben Hutchings
2010-01-27  5:05   ` David Miller

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