Netdev List
 help / color / mirror / Atom feed
* RE: [PATCH 06/26] bna: remove redundant NULL test before release_firmware()
From: Rasesh Mody @ 2012-04-09 21:14 UTC (permalink / raw)
  To: Jesper Juhl, linux-kernel@vger.kernel.org
  Cc: trivial@kernel.org, netdev@vger.kernel.org
In-Reply-To: <alpine.LNX.2.00.1204092210470.13925@swampdragon.chaosbits.net>

>From: Jesper Juhl [mailto:jj@chaosbits.net]
>Sent: Monday, April 09, 2012 1:50 PM
>To: linux-kernel@vger.kernel.org
>
>release_firmware() does its own NULL test so explicit test before call
>is unneeded.
>
>Signed-off-by: Jesper Juhl <jj@chaosbits.net>

Thanks Jesper!

Acked-by: Rasesh Mody <rmody@brocade.com>

^ permalink raw reply

* Re: [PATCH 04/26] typhoon: get rid of redundant conditional before all to release_firmware()
From: David Dillow @ 2012-04-09 21:03 UTC (permalink / raw)
  To: Jesper Juhl; +Cc: linux-kernel, trivial, netdev
In-Reply-To: <alpine.LNX.2.00.1204092208140.13925@swampdragon.chaosbits.net>

On Mon, 2012-04-09 at 22:50 +0200, Jesper Juhl wrote:
> There's no need to test for a NULL pointer before calling
> release_firmware() since the function does that check itself, so
> remove the redundant test.
> 
> Signed-off-by: Jesper Juhl <jj@chaosbits.net>

FWIW on a trivial patch,

Acked-by: David Dillow <dave@thedillows.org>

^ permalink raw reply

* RE: [PATCH 14/26] wireless, mwifiex: drop redundant NULL test before call to release_firmware()
From: Bing Zhao @ 2012-04-09 20:59 UTC (permalink / raw)
  To: Jesper Juhl, linux-kernel@vger.kernel.org
  Cc: trivial@kernel.org, netdev@vger.kernel.org,
	linux-wireless@vger.kernel.org, John W. Linville
In-Reply-To: <alpine.LNX.2.00.1204092224080.13925@swampdragon.chaosbits.net>

Hi Jesper,

Thanks for the patch.

> Since release_firmware() does its own test for NULL it is redundant to
> do so before calling it.
> 
> Signed-off-by: Jesper Juhl <jj@chaosbits.net>

Acked-by: Bing Zhao <bzhao@marvell.com>

Thanks,
Bing

> ---
>  drivers/net/wireless/mwifiex/main.c |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/mwifiex/main.c b/drivers/net/wireless/mwifiex/main.c
> index 9d1b3ca..2ee6162 100644
> --- a/drivers/net/wireless/mwifiex/main.c
> +++ b/drivers/net/wireless/mwifiex/main.c
> @@ -342,8 +342,7 @@ static int mwifiex_init_hw_fw(struct mwifiex_adapter *adapter)
>  	ret = 0;
> 
>  done:
> -	if (adapter->firmware)
> -		release_firmware(adapter->firmware);
> +	release_firmware(adapter->firmware);
>  	if (ret)
>  		ret = -1;
>  	return ret;
> --
> 1.7.10
> 
> 
> --
> Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
> Don't top-post http://www.catb.org/jargon/html/T/top-post.html
> Plain text mails only, please.

^ permalink raw reply

* Re: [PATCH v17 15/15] Documentation: prctl/seccomp_filter
From: Ryan Ware @ 2012-04-09 20:58 UTC (permalink / raw)
  To: Markus Gutschke, Will Drewry
  Cc: Andrew Morton, linux-kernel, linux-security-module, linux-arch,
	linux-doc, kernel-hardening, netdev, x86, arnd, davem, hpa, mingo,
	oleg, peterz, rdunlap, mcgrathr, tglx, luto, eparis, serge.hallyn,
	djm, scarybeasts, indan, pmoore, corbet, eric.dumazet, coreyb,
	keescook, jmorris
In-Reply-To: <CAE6n16kMv5Y+Cp9XZ=tmK0sgoJJZ6CJUq_ttuNEUMb4JFXJ33g@mail.gmail.com>

I second this!  They are extremely useful.

Ryan

On 4/9/12 1:47 PM, "Markus Gutschke" <markus@chromium.org> wrote:

>No matter what you do, please leave the samples accessible somewhere.
>They proved incredibly useful in figuring out how the API works. I am
>sure, other developers are going to appreciate them as well.
>
>Alternatively, if you don't want to include the samples with the
>kernel sources, figure out how you can include a sample in the
>official manual page for prctl().
>
>
>Markus
>--
>To unsubscribe from this list: send the line "unsubscribe
>linux-security-module" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html



^ permalink raw reply

* [PATCH 26/26] ipw2100: remove a redundant NULL check before calling release_firmware()
From: Jesper Juhl @ 2012-04-09 20:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: trivial, netdev, linux-wireless, John W. Linville,
	Stanislav Yakovlev
In-Reply-To: <alpine.LNX.2.00.1204092157340.13925@swampdragon.chaosbits.net>

The release_firmware() function does its own NULL test so a test
before calling it is rather redundant.

Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
 drivers/net/wireless/ipw2x00/ipw2100.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ipw2x00/ipw2100.c b/drivers/net/wireless/ipw2x00/ipw2100.c
index f0551f8..d8d804e 100644
--- a/drivers/net/wireless/ipw2x00/ipw2100.c
+++ b/drivers/net/wireless/ipw2x00/ipw2100.c
@@ -8508,8 +8508,7 @@ static void ipw2100_release_firmware(struct ipw2100_priv *priv,
 				     struct ipw2100_fw *fw)
 {
 	fw->version = 0;
-	if (fw->fw_entry)
-		release_firmware(fw->fw_entry);
+	release_firmware(fw->fw_entry);
 	fw->fw_entry = NULL;
 }
 
-- 
1.7.10


-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

^ permalink raw reply related

* [PATCH 15/26] wireless, orinoco: release_firmware() tests for NULL, remove explicit tests before calls
From: Jesper Juhl @ 2012-04-09 20:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: trivial, netdev, linux-wireless, Pavel Roskin, Paul Gortmaker,
	John W. Linville
In-Reply-To: <alpine.LNX.2.00.1204092157340.13925@swampdragon.chaosbits.net>

It is redundant to test for NULL pointers before calling
release_firmware() since the function does its own NULL test.

Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
 drivers/net/wireless/orinoco/fw.c |    7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/orinoco/fw.c b/drivers/net/wireless/orinoco/fw.c
index 4df8cf6..400a352 100644
--- a/drivers/net/wireless/orinoco/fw.c
+++ b/drivers/net/wireless/orinoco/fw.c
@@ -379,11 +379,8 @@ void orinoco_cache_fw(struct orinoco_private *priv, int ap)
 
 void orinoco_uncache_fw(struct orinoco_private *priv)
 {
-	if (priv->cached_pri_fw)
-		release_firmware(priv->cached_pri_fw);
-	if (priv->cached_fw)
-		release_firmware(priv->cached_fw);
-
+	release_firmware(priv->cached_pri_fw);
+	release_firmware(priv->cached_fw);
 	priv->cached_pri_fw = NULL;
 	priv->cached_fw = NULL;
 }
-- 
1.7.10


-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

^ permalink raw reply related

* [PATCH 14/26] wireless, mwifiex: drop redundant NULL test before call to release_firmware()
From: Jesper Juhl @ 2012-04-09 20:51 UTC (permalink / raw)
  To: linux-kernel; +Cc: trivial, netdev, linux-wireless, John W. Linville, Bing Zhao
In-Reply-To: <alpine.LNX.2.00.1204092157340.13925@swampdragon.chaosbits.net>

Since release_firmware() does its own test for NULL it is redundant to
do so before calling it.

Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
 drivers/net/wireless/mwifiex/main.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/wireless/mwifiex/main.c b/drivers/net/wireless/mwifiex/main.c
index 9d1b3ca..2ee6162 100644
--- a/drivers/net/wireless/mwifiex/main.c
+++ b/drivers/net/wireless/mwifiex/main.c
@@ -342,8 +342,7 @@ static int mwifiex_init_hw_fw(struct mwifiex_adapter *adapter)
 	ret = 0;
 
 done:
-	if (adapter->firmware)
-		release_firmware(adapter->firmware);
+	release_firmware(adapter->firmware);
 	if (ret)
 		ret = -1;
 	return ret;
-- 
1.7.10


-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

^ permalink raw reply related

* [PATCH 13/26] wireless, libertas: remove redundant NULL tests before calling release_firmware()
From: Jesper Juhl @ 2012-04-09 20:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: trivial, netdev, linux-wireless, libertas-dev, John W. Linville,
	Dan Williams
In-Reply-To: <alpine.LNX.2.00.1204092157340.13925@swampdragon.chaosbits.net>

release_firmware() tests for, and deals gracefully with, NULL
pointers. Remove redundant explicit tests before calling the function.

Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
 drivers/net/wireless/libertas/if_cs.c   |    6 ++----
 drivers/net/wireless/libertas/if_sdio.c |    6 ++----
 drivers/net/wireless/libertas/if_spi.c  |    6 ++----
 drivers/net/wireless/libertas/main.c    |   12 ++++--------
 4 files changed, 10 insertions(+), 20 deletions(-)

diff --git a/drivers/net/wireless/libertas/if_cs.c b/drivers/net/wireless/libertas/if_cs.c
index 234ee88..171a06b 100644
--- a/drivers/net/wireless/libertas/if_cs.c
+++ b/drivers/net/wireless/libertas/if_cs.c
@@ -951,10 +951,8 @@ out2:
 out1:
 	pcmcia_disable_device(p_dev);
 out:
-	if (helper)
-		release_firmware(helper);
-	if (mainfw)
-		release_firmware(mainfw);
+	release_firmware(helper);
+	release_firmware(mainfw);
 
 	lbs_deb_leave_args(LBS_DEB_CS, "ret %d", ret);
 	return ret;
diff --git a/drivers/net/wireless/libertas/if_sdio.c b/drivers/net/wireless/libertas/if_sdio.c
index 9804ebc..15bfe2f 100644
--- a/drivers/net/wireless/libertas/if_sdio.c
+++ b/drivers/net/wireless/libertas/if_sdio.c
@@ -751,10 +751,8 @@ success:
 	ret = 0;
 
 out:
-	if (helper)
-		release_firmware(helper);
-	if (mainfw)
-		release_firmware(mainfw);
+	release_firmware(helper);
+	release_firmware(mainfw);
 
 	lbs_deb_leave_args(LBS_DEB_SDIO, "ret %d", ret);
 	return ret;
diff --git a/drivers/net/wireless/libertas/if_spi.c b/drivers/net/wireless/libertas/if_spi.c
index 50b1ee7..7a5df4f 100644
--- a/drivers/net/wireless/libertas/if_spi.c
+++ b/drivers/net/wireless/libertas/if_spi.c
@@ -1095,10 +1095,8 @@ static int if_spi_init_card(struct if_spi_card *card)
 		goto out;
 
 out:
-	if (helper)
-		release_firmware(helper);
-	if (mainfw)
-		release_firmware(mainfw);
+	release_firmware(helper);
+	release_firmware(mainfw);
 
 	lbs_deb_leave_args(LBS_DEB_SPI, "err %d\n", err);
 
diff --git a/drivers/net/wireless/libertas/main.c b/drivers/net/wireless/libertas/main.c
index 957681d..3b81b70 100644
--- a/drivers/net/wireless/libertas/main.c
+++ b/drivers/net/wireless/libertas/main.c
@@ -1269,14 +1269,10 @@ int lbs_get_firmware(struct device *dev, const char *user_helper,
 
   fail:
 	/* Failed */
-	if (*helper) {
-		release_firmware(*helper);
-		*helper = NULL;
-	}
-	if (*mainfw) {
-		release_firmware(*mainfw);
-		*mainfw = NULL;
-	}
+	release_firmware(*helper);
+	*helper = NULL;
+	release_firmware(*mainfw);
+	*mainfw = NULL;
 
 	return -ENOENT;
 }
-- 
1.7.10


-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

^ permalink raw reply related

* [PATCH 12/26] ipw2200: remove a redundant NULL check before calling release_firmware()
From: Jesper Juhl @ 2012-04-09 20:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: trivial, netdev, linux-wireless, John W. Linville,
	Stanislav Yakovlev
In-Reply-To: <alpine.LNX.2.00.1204092157340.13925@swampdragon.chaosbits.net>

The release_firmware() function does its own NULL test, so testing
before calling it is rather redundant.

Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
 drivers/net/wireless/ipw2x00/ipw2200.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ipw2x00/ipw2200.c b/drivers/net/wireless/ipw2x00/ipw2200.c
index 2b02257..77c5d2f 100644
--- a/drivers/net/wireless/ipw2x00/ipw2200.c
+++ b/drivers/net/wireless/ipw2x00/ipw2200.c
@@ -3657,8 +3657,7 @@ static int ipw_load(struct ipw_priv *priv)
 		priv->rxq = NULL;
 	}
 	ipw_tx_queue_free(priv);
-	if (raw)
-		release_firmware(raw);
+	release_firmware(raw);
 #ifdef CONFIG_PM
 	fw_loaded = 0;
 	raw = NULL;
-- 
1.7.10


-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

^ permalink raw reply related

* [PATCH 11/26] wireless, atmel: remove pointless test for NULL before release_firmware() call
From: Jesper Juhl @ 2012-04-09 20:50 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: trivial-DgEjT+Ai2ygdnm+yROfE0A, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA, John W. Linville,
	Simon Kelley
In-Reply-To: <alpine.LNX.2.00.1204092157340.13925-h2p7t3/P30RzeRGmFJ5qR7ZzlVVXadcDXqFh9Ls21Oc@public.gmane.org>

release_firmware() does its own test. Explicitly checking before the
call is redundant.

Signed-off-by: Jesper Juhl <jj-IYz4IdjRLj0sV2N9l4h3zg@public.gmane.org>
---
 drivers/net/wireless/atmel.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/wireless/atmel.c b/drivers/net/wireless/atmel.c
index 6c87a82..d07c030 100644
--- a/drivers/net/wireless/atmel.c
+++ b/drivers/net/wireless/atmel.c
@@ -3989,8 +3989,7 @@ static int reset_atmel_card(struct net_device *dev)
 			atmel_copy_to_card(priv->dev, 0x8000, &fw[0x6000], len - 0x6000);
 		}
 
-		if (fw_entry)
-			release_firmware(fw_entry);
+		release_firmware(fw_entry);
 	}
 
 	err = atmel_wakeup_firmware(priv);
-- 
1.7.10


-- 
Jesper Juhl <jj-IYz4IdjRLj0sV2N9l4h3zg@public.gmane.org>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 10/26] wireless, at76c50x:: Don't needlessly test for NULL before calling release_firmware()
From: Jesper Juhl @ 2012-04-09 20:50 UTC (permalink / raw)
  To: linux-kernel; +Cc: trivial, netdev, linux-wireless, John W. Linville
In-Reply-To: <alpine.LNX.2.00.1204092157340.13925@swampdragon.chaosbits.net>

The release_firmware() function deals gracefully with being passed a
NULL pointer, so explicit tests before the call are rather pointless.

Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
 drivers/net/wireless/at76c50x-usb.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/at76c50x-usb.c b/drivers/net/wireless/at76c50x-usb.c
index 4045e5a..faa8bcb 100644
--- a/drivers/net/wireless/at76c50x-usb.c
+++ b/drivers/net/wireless/at76c50x-usb.c
@@ -2512,10 +2512,8 @@ static void __exit at76_mod_exit(void)
 
 	printk(KERN_INFO DRIVER_DESC " " DRIVER_VERSION " unloading\n");
 	usb_deregister(&at76_driver);
-	for (i = 0; i < ARRAY_SIZE(firmwares); i++) {
-		if (firmwares[i].fw)
-			release_firmware(firmwares[i].fw);
-	}
+	for (i = 0; i < ARRAY_SIZE(firmwares); i++)
+		release_firmware(firmwares[i].fw);
 	led_trigger_unregister_simple(ledtrig_tx);
 }
 
-- 
1.7.10


-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

^ permalink raw reply related

* [PATCH 09/26] tehuti: delete redundant NULL check before release_firmware()
From: Jesper Juhl @ 2012-04-09 20:50 UTC (permalink / raw)
  To: linux-kernel; +Cc: trivial, netdev, Andy Gospodarek
In-Reply-To: <alpine.LNX.2.00.1204092157340.13925@swampdragon.chaosbits.net>

release_firmware() checks for NULL pointers - no need to test before
the call.

Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
 drivers/net/ethernet/tehuti/tehuti.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/tehuti/tehuti.c b/drivers/net/ethernet/tehuti/tehuti.c
index ad973ff..a445e77 100644
--- a/drivers/net/ethernet/tehuti/tehuti.c
+++ b/drivers/net/ethernet/tehuti/tehuti.c
@@ -341,8 +341,8 @@ static int bdx_fw_load(struct bdx_priv *priv)
 out:
 	if (master)
 		WRITE_REG(priv, regINIT_SEMAPHORE, 1);
-	if (fw)
-		release_firmware(fw);
+
+	release_firmware(fw);
 
 	if (rc) {
 		netdev_err(priv->ndev, "firmware loading failed\n");
-- 
1.7.10


-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

^ permalink raw reply related

* [PATCH 08/26] qlogic, qlcnic: get rid of redundant test for NULL before a call to release_firmware()
From: Jesper Juhl @ 2012-04-09 20:50 UTC (permalink / raw)
  To: linux-kernel
  Cc: trivial, netdev, linux-driver, Sony Chacko, Anirban Chakraborty
In-Reply-To: <alpine.LNX.2.00.1204092157340.13925@swampdragon.chaosbits.net>

Since release_firmware() deals gracefully with being passed a NULL
pointer there's no reason to test explicitly before calling the
function.

Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c
index d32cf0d..799fd40 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c
@@ -1321,8 +1321,7 @@ next:
 void
 qlcnic_release_firmware(struct qlcnic_adapter *adapter)
 {
-	if (adapter->fw)
-		release_firmware(adapter->fw);
+	release_firmware(adapter->fw);
 	adapter->fw = NULL;
 }
 
-- 
1.7.10


-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

^ permalink raw reply related

* [PATCH 07/26] qlogic, netxen: get rid of a redundant test for NULL before call to release_firmware()
From: Jesper Juhl @ 2012-04-09 20:50 UTC (permalink / raw)
  To: linux-kernel; +Cc: trivial, netdev, Rajesh Borundia, Sony Chacko
In-Reply-To: <alpine.LNX.2.00.1204092157340.13925@swampdragon.chaosbits.net>

Since release_firmware() deals gracefully with being passed a NULL
pointer there is no reason to test explicitly before calling the
function.

Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
 drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
index 718b274..1fb149c 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
@@ -1261,8 +1261,7 @@ next:
 void
 netxen_release_firmware(struct netxen_adapter *adapter)
 {
-	if (adapter->fw)
-		release_firmware(adapter->fw);
+	release_firmware(adapter->fw);
 	adapter->fw = NULL;
 }
 
-- 
1.7.10


-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

^ permalink raw reply related

* [PATCH 06/26] bna: remove redundant NULL test before release_firmware()
From: Jesper Juhl @ 2012-04-09 20:50 UTC (permalink / raw)
  To: linux-kernel; +Cc: trivial, netdev, Rasesh Mody
In-Reply-To: <alpine.LNX.2.00.1204092157340.13925@swampdragon.chaosbits.net>

release_firmware() does its own NULL test so explicit test before call
is unneeded.

Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
 drivers/net/ethernet/brocade/bna/bnad.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/brocade/bna/bnad.c b/drivers/net/ethernet/brocade/bna/bnad.c
index ff78f77..d86390c 100644
--- a/drivers/net/ethernet/brocade/bna/bnad.c
+++ b/drivers/net/ethernet/brocade/bna/bnad.c
@@ -3546,9 +3546,7 @@ static void __exit
 bnad_module_exit(void)
 {
 	pci_unregister_driver(&bnad_pci_driver);
-
-	if (bfi_fw)
-		release_firmware(bfi_fw);
+	release_firmware(bfi_fw);
 }
 
 module_init(bnad_module_init);
-- 
1.7.10


-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

^ permalink raw reply related

* [PATCH 05/26] tg3: remove redundant NULL test before release_firmware() call
From: Jesper Juhl @ 2012-04-09 20:50 UTC (permalink / raw)
  To: linux-kernel; +Cc: trivial, netdev, Michael Chan, Matt Carlson
In-Reply-To: <alpine.LNX.2.00.1204092157340.13925@swampdragon.chaosbits.net>

There is no need to test for a NULL pointer before calling
release_firmware - the function does that on its own.

Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
 drivers/net/ethernet/broadcom/tg3.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 062ac33..9fbf73e 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -15842,8 +15842,7 @@ static void __devexit tg3_remove_one(struct pci_dev *pdev)
 	if (dev) {
 		struct tg3 *tp = netdev_priv(dev);
 
-		if (tp->fw)
-			release_firmware(tp->fw);
+		release_firmware(tp->fw);
 
 		tg3_reset_task_cancel(tp);
 
-- 
1.7.10


-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

^ permalink raw reply related

* [PATCH 04/26] typhoon: get rid of redundant conditional before all to release_firmware()
From: Jesper Juhl @ 2012-04-09 20:50 UTC (permalink / raw)
  To: linux-kernel; +Cc: trivial, netdev, David Dillow
In-Reply-To: <alpine.LNX.2.00.1204092157340.13925@swampdragon.chaosbits.net>

There's no need to test for a NULL pointer before calling
release_firmware() since the function does that check itself, so
remove the redundant test.

Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
 drivers/net/ethernet/3com/typhoon.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/3com/typhoon.c b/drivers/net/ethernet/3com/typhoon.c
index 1234a14..b153666 100644
--- a/drivers/net/ethernet/3com/typhoon.c
+++ b/drivers/net/ethernet/3com/typhoon.c
@@ -2549,8 +2549,7 @@ typhoon_init(void)
 static void __exit
 typhoon_cleanup(void)
 {
-	if (typhoon_fw)
-		release_firmware(typhoon_fw);
+	release_firmware(typhoon_fw);
 	pci_unregister_driver(&typhoon_driver);
 }
 
-- 
1.7.10


-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

^ permalink raw reply related

* Re: [PATCH v17 15/15] Documentation: prctl/seccomp_filter
From: Markus Gutschke @ 2012-04-09 20:47 UTC (permalink / raw)
  To: Will Drewry
  Cc: Andrew Morton, linux-kernel, linux-security-module, linux-arch,
	linux-doc, kernel-hardening, netdev, x86, arnd, davem, hpa, mingo,
	oleg, peterz, rdunlap, mcgrathr, tglx, luto, eparis, serge.hallyn,
	djm, scarybeasts, indan, pmoore, corbet, eric.dumazet, coreyb,
	keescook, jmorris
In-Reply-To: <CABqD9haT5h-7LMkbGSaQbnk8SJj9L21GT61UZCMVWLJCTw9OBg@mail.gmail.com>

No matter what you do, please leave the samples accessible somewhere.
They proved incredibly useful in figuring out how the API works. I am
sure, other developers are going to appreciate them as well.

Alternatively, if you don't want to include the samples with the
kernel sources, figure out how you can include a sample in the
official manual page for prctl().


Markus

^ permalink raw reply

* Re: [PATCH v2 2/2] cgroup: get rid of populate for memcg
From: Tejun Heo @ 2012-04-09 20:39 UTC (permalink / raw)
  To: Glauber Costa
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, cgroups-u79uwXL29TY76Z2rM5mHXA,
	kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A, Li Zefan, Johannes Weiner,
	Michal Hocko, Balbir Singh
In-Reply-To: <4F83442C.8040006-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>

Hello, Glauber.

On Mon, Apr 09, 2012 at 05:18:52PM -0300, Glauber Costa wrote:
> After debugging this a bit, I found most of it not to be a problem.
> Unless I am *very* wrong (and I both read and tested stuff), mount
> operations do not recreate the root cgroup. So stuff like the hotcpu
> notifier, etc, won't be a problem.

Ooh, right, root css is created during subsys init.  Please note that
the kernel will panic on subsys init failure (hmm....).

> Now, you are definitely correct in pointing out that we start
> leaking stuff now - my bad.
> 
> But I guess I can then bundle it in a new submission, after
> shuffling around stuff a bit. It is really not a bug now, so no
> reason to route it separately.

Sure, just make it a separate patch.  As long as memcg ppl are fine
with it, I can route the patches together.

Thanks.

-- 
tejun

^ permalink raw reply

* [PATCH net-next] rtnetlink: fix spelling errors
From: Stephen Hemminger @ 2012-04-09 20:27 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

Minor spelling mistakes.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

--- a/include/net/rtnetlink.h	2012-02-27 08:43:02.400935781 -0800
+++ b/include/net/rtnetlink.h	2012-04-09 13:24:02.705034830 -0700
@@ -41,7 +41,7 @@ static inline int rtnl_msg_family(const
  *	@get_size: Function to calculate required room for dumping device
  *		   specific netlink attributes
  *	@fill_info: Function to dump device specific netlink attributes
- *	@get_xstats_size: Function to calculate required room for dumping devic
+ *	@get_xstats_size: Function to calculate required room for dumping device
  *			  specific statistics
  *	@fill_xstats: Function to dump device specific statistics
  */
@@ -94,7 +94,7 @@ extern void	rtnl_link_unregister(struct
  * 	@fill_link_af: Function to fill IFLA_AF_SPEC with address family
  * 		       specific netlink attributes.
  * 	@get_link_af_size: Function to calculate size of address family specific
- * 			   netlink attributes exlusive the container attribute.
+ * 			   netlink attributes exclusive the container attribute.
  *	@validate_link_af: Validate a IFLA_AF_SPEC attribute, must check attr
  *			   for invalid configuration settings.
  * 	@set_link_af: Function to parse a IFLA_AF_SPEC attribute and modify

^ permalink raw reply

* Re: [PATCH v2 2/2] cgroup: get rid of populate for memcg
From: Glauber Costa @ 2012-04-09 20:18 UTC (permalink / raw)
  To: Tejun Heo
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, cgroups-u79uwXL29TY76Z2rM5mHXA,
	kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A, Li Zefan, Johannes Weiner,
	Michal Hocko, Balbir Singh
In-Reply-To: <20120409181331.GD7522-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>

On 04/09/2012 03:13 PM, Tejun Heo wrote:
> Hello,
>
> On Mon, Apr 09, 2012 at 02:51:10PM -0300, Glauber Costa wrote:
>> On 04/09/2012 02:40 PM, Tejun Heo wrote:
>>> which BTW seems incorrect even on its
>>> own - unmounting and mounting again would probably make the same
>>> notifier registered multiple times corrupting notification chain, and
>>> ref inc on the parent.
>>
>>
>> For the maintainers: Should I fix those in a new submission, or do
>> you intend to do it yourselves?
>>
>> the refcnt dropping should probably be done in my patch, it is a new
>> leak (sorry). The hotplug notifier, as tejun pointed, was already
>> there.
>>
>> It seems simple enough to fix, so if you guys want, I can bundle it in
>> a new submission.
>
> I think it would be best to create a separate patch which is routed
> through the usual memcg path (I suppose memcg patches go through
> -mm?).
>
> Thanks.
>
Tejun,

After debugging this a bit, I found most of it not to be a problem. 
Unless I am *very* wrong (and I both read and tested stuff), mount 
operations do not recreate the root cgroup. So stuff like the hotcpu 
notifier, etc, won't be a problem.

Now, you are definitely correct in pointing out that we start leaking 
stuff now - my bad.

But I guess I can then bundle it in a new submission, after shuffling 
around stuff a bit. It is really not a bug now, so no reason to route it 
separately.

^ permalink raw reply

* Re: [PATCH v17 08/15] seccomp: add system call filtering using BPF
From: Will Drewry @ 2012-04-09 19:59 UTC (permalink / raw)
  To: Indan Zupancic
  Cc: Andrew Morton, linux-kernel, linux-security-module, linux-arch,
	linux-doc, kernel-hardening, netdev, x86, arnd, davem, hpa, mingo,
	oleg, peterz, rdunlap, mcgrathr, tglx, luto, eparis, serge.hallyn,
	djm, scarybeasts, pmoore, corbet, eric.dumazet, markus, coreyb,
	keescook, jmorris
In-Reply-To: <67e30a0c8655fc53a92e8138bba9de66.squirrel@webmail.greenhost.nl>

On Sun, Apr 8, 2012 at 1:22 PM, Indan Zupancic <indan@nul.nu> wrote:
> On Sat, April 7, 2012 06:23, Andrew Morton wrote:
>> hm, I'm surprised that we don't have a zero-returning implementation of
>> is_compat_task() when CONFIG_COMPAT=n.  Seems silly.  Blames Arnd.
>
> It's sneakily hidden at the end of compat.h.
>
>>> +/**
>>> + * get_u32 - returns a u32 offset into data
>>> + * @data: a unsigned 64 bit value
>>> + * @index: 0 or 1 to return the first or second 32-bits
>>> + *
>>> + * This inline exists to hide the length of unsigned long.
>>> + * If a 32-bit unsigned long is passed in, it will be extended
>>> + * and the top 32-bits will be 0. If it is a 64-bit unsigned
>>> + * long, then whatever data is resident will be properly returned.
>>> + */
>>> +static inline u32 get_u32(u64 data, int index)
>>> +{
>>> +    return ((u32 *)&data)[index];
>>> +}
>>
>> This seems utterly broken on big-endian machines.  If so: fix.  If not:
>> add comment explaining why?
>
> It's not a bug, it's intentional.
>
> I tried to convince them to have a stable ABI for all archs, but they
> didn't want to make the ABI endianness independent, because it looks
> uglier. The argument being that system call numbers are arch dependent
> anyway.
>
> So a filter for a little endian arch needs to check a different offset
> than one for a big endian archs.

Awkward, but in practice it doesn't seem to matter either way --
especially since properly written filters should check the @arch which
indicates the calling convention, endianness, etc.

>>> +static u32 seccomp_run_filters(int syscall)
>>> +{
>>> +    struct seccomp_filter *f;
>>> +    u32 ret = SECCOMP_RET_KILL;
>>> +    /*
>>> +     * All filters are evaluated in order of youngest to oldest. The lowest
>>> +     * BPF return value always takes priority.
>>> +     */
>>
>> The youngest-first design surprised me.  It wasn't mentioned at all in
>> the changelog.  Thinking about it, I guess it just doesn't matter.  But
>> some description of the reasons for and implications of this decision
>> for the uninitiated would be welcome.
>
> I think it's less confusing to not mention the order at all, exactly
> because it doesn't matter. It has been like this from the start, so
> that's why it's not mentioned in the changelog I guess.

Good call - I will remove that comment.  The only relevant information
is that the lowest return value wins.  I did add a comment up near
struct seccomp_filter with my attempt at explaining the tree
structure, but I didn't detail evaluation order.  In this case, it
only is relevant because our only link to the tree is via our local
pointer which happens to be the "youngest".

> The reason to check the youngest first is because the filters are shared
> between processes: childs inherit it. If later on additional filters are
> added, the only way of adding them without modifying an older filter is
> by adding them to the head of the list. This way no locking is needed,
> because filters are only added and removed single-threadedly, and never
> modified when being shared.

I tried a handful of other strategies, but in practice this seemed to
meet the needs with the least complexity/overhead.

>>> +/**
>>> + * seccomp_attach_filter: Attaches a seccomp filter to current.
>>> + * @fprog: BPF program to install
>>> + *
>>> + * Returns 0 on success or an errno on failure.
>>> + */
>>> +static long seccomp_attach_filter(struct sock_fprog *fprog)
>>> +{
>>> +    struct seccomp_filter *filter;
>>> +    unsigned long fp_size = fprog->len * sizeof(struct sock_filter);
>>> +    unsigned long total_insns = fprog->len;
>>> +    long ret;
>>> +
>>> +    if (fprog->len == 0 || fprog->len > BPF_MAXINSNS)
>>> +            return -EINVAL;
>>> +
>>> +    for (filter = current->seccomp.filter; filter; filter = filter->prev)
>>> +            total_insns += filter->len + 4;  /* include a 4 instr penalty */
>>
>> So tasks don't share filters?  We copy them by value at fork?  Do we do
>> this at vfork() too?
>
> Yes they do. But shared filters are never modified, except for the refcount.
>
>>
>>> +    if (total_insns > MAX_INSNS_PER_PATH)
>>> +            return -ENOMEM;
>>> +
>>> +    /*
>>> +     * Installing a seccomp filter requires that the task have
>>> +     * CAP_SYS_ADMIN in its namespace or be running with no_new_privs.
>>> +     * This avoids scenarios where unprivileged tasks can affect the
>>> +     * behavior of privileged children.
>>> +     */
>>> +    if (!current->no_new_privs &&
>>> +        security_capable_noaudit(current_cred(), current_user_ns(),
>>> +                                 CAP_SYS_ADMIN) != 0)
>>> +            return -EACCES;
>>> +
>>> +    /* Allocate a new seccomp_filter */
>>> +    filter = kzalloc(sizeof(struct seccomp_filter) + fp_size, GFP_KERNEL);
>>
>> I think this gives userspace an easy way of causing page allocation
>> failure warnings, by permitting large kmalloc() attempts.  Add
>> __GFP_NOWARN?
>
> Max is 32kb. sk_attach_filter() in net/core/filter.c is worse,
> it allocates up to 512kb before even checking the length.
>
> What about using GFP_USER (and adding __GFP_NOWARN to GFP_USER) instead?

It looks like GFP_USER|__GFP_NOWARN would make sense here.  I'll change it.

>>> +    /* Check and rewrite the fprog via the skb checker */
>>> +    ret = sk_chk_filter(filter->insns, filter->len);
>>> +    if (ret)
>>> +            goto fail;
>>> +
>>> +    /* Check and rewrite the fprog for seccomp use */
>>> +    ret = seccomp_chk_filter(filter->insns, filter->len);
>>
>> "check" is spelled "check"!
>
> Yes, it is and he did spell "check" as "Check".
>
> seccomp_chk_filter() mirrors sk_chk_filter(). So it refers to
> "chk", not "check".

I can change it to be written out or leave it matching the networking
code.  Any preferences?

Thanks!
will

^ permalink raw reply

* Re: 3.2.8/amd64 full interrupt hangs and deadlocks under big network copies (page allocation failure)
From: Marc MERLIN @ 2012-04-09 19:46 UTC (permalink / raw)
  To: David Miller
  Cc: eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
	Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ,
	bhutchings-s/n/eUQHGBpZroRs9YW3xA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20120409.153452.1284163346306246866.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>

On Mon, Apr 09, 2012 at 03:34:52PM -0400, David Miller wrote:
> From: Eric Dumazet <eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Date: Mon, 09 Apr 2012 21:11:12 +0200
> 
> > I think Marc posted stack traces showing problem on transmit side.
>  ...
> > I dont really understand how it can happen, with MTU=1500
> 
> Depending upon the configuration and the driver, wireless can need
> more headroom.  For encryption an extra 8 bytes are necessary, and the
> driver may request a variable amount of extra headroom via
> ->hw.extra_tx_headroom
> 
> What wireless device are we dealing with again?

Intel(R) Wireless WiFi Link AGN driver for Linux, in-tree:
Copyright(c) 2003-2011 Intel Corporation
iwlwifi 0000:03:00.0: PCI INT A -> GSI 17 (level, low) -> IRQ 17
iwlwifi 0000:03:00.0: setting latency timer to 64
iwlwifi 0000:03:00.0: pci_resource_len = 0x00002000
iwlwifi 0000:03:00.0: pci_resource_base = ffffc900057a4000
iwlwifi 0000:03:00.0: HW Revision ID = 0x0
iwlwifi 0000:03:00.0: irq 47 for MSI/MSI-X
iwlwifi 0000:03:00.0: Detected Intel(R) Ultimate N WiFi Link 5300 AGN, REV=0x24
iwlwifi 0000:03:00.0: L1 Disabled; Enabling L0S
iwlwifi 0000:03:00.0: device EEPROM VER=0x11e, CALIB=0x4
iwlwifi 0000:03:00.0: Device SKU: 0Xf0
iwlwifi 0000:03:00.0: Tunable channels: 13 802.11bg, 24 802.11a channels

along with iwlwifi-5000-5.ucode

Marc
-- 
"A mouse is a device used to point at the xterm you want to type in" - A.S.R.
Microsoft is to operating systems ....
                                      .... what McDonalds is to gourmet cooking
Home page: http://marc.merlins.org/
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v17 15/15] Documentation: prctl/seccomp_filter
From: Will Drewry @ 2012-04-09 19:46 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-kernel, linux-security-module, linux-arch, linux-doc,
	kernel-hardening, netdev, x86, arnd, davem, hpa, mingo, oleg,
	peterz, rdunlap, mcgrathr, tglx, luto, eparis, serge.hallyn, djm,
	scarybeasts, indan, pmoore, corbet, eric.dumazet, markus, coreyb,
	keescook, jmorris
In-Reply-To: <20120406142644.223ffff0.akpm@linux-foundation.org>

On Fri, Apr 6, 2012 at 4:26 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Thu, 29 Mar 2012 15:02:00 -0500
> Will Drewry <wad@chromium.org> wrote:
>
>> Documents how system call filtering using Berkeley Packet
>> Filter programs works and how it may be used.
>> Includes an example for x86 and a semi-generic
>> example using a macro-based code generator.
>>
>>
>> ...
>>
>> +Adding architecture support
>> +-----------------------
>> +
>> +See arch/Kconfig for the authoritative requirements.  In general, if an
>> +architecture supports both ptrace_event and seccomp, it will be able to
>> +support seccomp filter with minor fixup: SIGSYS support and seccomp return
>> +value checking.  Then it must just add CONFIG_HAVE_ARCH_SECCOMP_FILTER
>> +to its arch-specific Kconfig.
>> diff --git a/samples/Makefile b/samples/Makefile
>> index 2f75851..5ef08bb 100644
>> --- a/samples/Makefile
>> +++ b/samples/Makefile
>
> Oh good, I was going to ask about that.
>
> Can we get this code into tools/testing/selftests?  That way people
> will run it more often and it's more likely to be maintained as the
> code evolves.

I'm currently using a lightweight testsuite in addition to the
samples.  It's a little more oriented at pass/fail behavior.  Would it
be more appropriate to post those in addition to, or instead of,
samples?

thanks!

^ permalink raw reply

* Re: [kernel-hardening] Re: [PATCH v17 09/15] seccomp: remove duplicated failure logging
From: Kees Cook @ 2012-04-09 19:39 UTC (permalink / raw)
  To: Eric Paris
  Cc: kernel-hardening, Andrew Morton, linux-kernel,
	linux-security-module, linux-arch, linux-doc, netdev, x86, arnd,
	davem, hpa, mingo, oleg, peterz, rdunlap, mcgrathr, tglx, luto,
	serge.hallyn, djm, scarybeasts, indan, pmoore, corbet,
	eric.dumazet, markus, coreyb, jmorris
In-Reply-To: <1333999988.14260.9.camel@localhost>

On Mon, Apr 9, 2012 at 12:33 PM, Eric Paris <eparis@redhat.com> wrote:
> On Mon, 2012-04-09 at 14:26 -0500, Will Drewry wrote:
>> On Fri, Apr 6, 2012 at 4:14 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
>> > On Thu, 29 Mar 2012 15:01:54 -0500
>> > Will Drewry <wad@chromium.org> wrote:
>
>> >> -void __audit_seccomp(unsigned long syscall)
>> >> +void __audit_seccomp(unsigned long syscall, long signr, int code)
>> >>  {
>> >>       struct audit_buffer *ab;
>> >>
>> >>       ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_ANOM_ABEND);
>> >> -     audit_log_abend(ab, "seccomp", SIGKILL);
>> >> +     audit_log_abend(ab, "seccomp", signr);
>> >>       audit_log_format(ab, " syscall=%ld", syscall);
>> >> +#ifdef CONFIG_COMPAT
>> >> +     audit_log_format(ab, " compat=%d", is_compat_task());
>> >> +#endif
>> >
>> > We don't need the ifdef for compilation reasons now.
>> >
>> > The question is: should we emit the compat= record on
>> > non-compat-capable architectures?  Doing so would be safer - making it
>> > conditional invites people to write x86-only usersapce.
>>
>> I'd certainly prefer it always being there for exactly that reason.
>>
>> Kees, Eric, any preferences?  Unless I hear one, I'll just drop the
>> ifdefs in the next revision.
>
> I'd just leave it in unconditionally.  The audit parse libraries would
> handle it just fine, but that doesn't mean everyone uses that tool to
> parse the text.

Related to this, can we get this patch into a tree as well?
https://lkml.org/lkml/2012/3/23/332

Thanks,

-Kees

-- 
Kees Cook
ChromeOS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox