* [PATCH wireless-next] brcmsmac: avoid 512 byte stack variable
@ 2013-02-07 20:13 Tim Gardner
2013-02-07 20:19 ` Joe Perches
2013-02-07 20:22 ` Arend van Spriel
0 siblings, 2 replies; 5+ messages in thread
From: Tim Gardner @ 2013-02-07 20:13 UTC (permalink / raw)
To: linux-kernel
Cc: Tim Gardner, Brett Rudley, Arend van Spriel,
Franky (Zhenhui) Lin, Hante Meuleman, John W. Linville,
Seth Forshee, Pieter-Paul Giesberts, Hauke Mehrtens,
linux-wireless, brcm80211-dev-list, netdev
Dynamically allocate the probe response template which
avoids potential stack corruption. Observed with smatch:
drivers/net/wireless/brcm80211/brcmsmac/main.c:7412 brcms_c_bss_update_probe_resp()
warn: 'prb_resp' puts 512 bytes on stack
Cc: Brett Rudley <brudley@broadcom.com>
Cc: Arend van Spriel <arend@broadcom.com>
Cc: "Franky (Zhenhui) Lin" <frankyl@broadcom.com>
Cc: Hante Meuleman <meuleman@broadcom.com>
Cc: "John W. Linville" <linville@tuxdriver.com>
Cc: Seth Forshee <seth.forshee@canonical.com>
Cc: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Cc: Hauke Mehrtens <hauke@hauke-m.de>
Cc: linux-wireless@vger.kernel.org
Cc: brcm80211-dev-list@broadcom.com
Cc: netdev@vger.kernel.org
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
drivers/net/wireless/brcm80211/brcmsmac/main.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/brcm80211/brcmsmac/main.c b/drivers/net/wireless/brcm80211/brcmsmac/main.c
index c26992a..e392e76 100644
--- a/drivers/net/wireless/brcm80211/brcmsmac/main.c
+++ b/drivers/net/wireless/brcm80211/brcmsmac/main.c
@@ -7408,9 +7408,16 @@ brcms_c_bss_update_probe_resp(struct brcms_c_info *wlc,
struct brcms_bss_cfg *cfg,
bool suspend)
{
- u16 prb_resp[BCN_TMPL_LEN / 2];
+ u16 *prb_resp;
int len = BCN_TMPL_LEN;
+ prb_resp = kmalloc(BCN_TMPL_LEN, GFP_ATOMIC);
+ if (!prb_resp) {
+ wiphy_err(wlc->wiphy, "wl: %s: failed to alloc %u bytes\n",
+ __func__, BCN_TMPL_LEN);
+ return;
+ }
+
/*
* write the probe response to hardware, or save in
* the config structure
@@ -7444,6 +7451,8 @@ brcms_c_bss_update_probe_resp(struct brcms_c_info *wlc,
if (suspend)
brcms_c_enable_mac(wlc);
+
+ kfree(prb_resp);
}
void brcms_c_update_probe_resp(struct brcms_c_info *wlc, bool suspend)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH wireless-next] brcmsmac: avoid 512 byte stack variable
2013-02-07 20:13 [PATCH wireless-next] brcmsmac: avoid 512 byte stack variable Tim Gardner
@ 2013-02-07 20:19 ` Joe Perches
2013-02-07 20:23 ` Arend van Spriel
2013-02-07 20:22 ` Arend van Spriel
1 sibling, 1 reply; 5+ messages in thread
From: Joe Perches @ 2013-02-07 20:19 UTC (permalink / raw)
To: Tim Gardner
Cc: linux-kernel, Brett Rudley, Arend van Spriel,
Franky (Zhenhui) Lin, Hante Meuleman, John W. Linville,
Seth Forshee, Pieter-Paul Giesberts, Hauke Mehrtens,
linux-wireless, brcm80211-dev-list, netdev
On Thu, 2013-02-07 at 13:13 -0700, Tim Gardner wrote:
> Dynamically allocate the probe response template which
> avoids potential stack corruption. Observed with smatch:
trivial:
> diff --git a/drivers/net/wireless/brcm80211/brcmsmac/main.c b/drivers/net/wireless/brcm80211/brcmsmac/main.c
[]
> @@ -7408,9 +7408,16 @@ brcms_c_bss_update_probe_resp(struct brcms_c_info *wlc,
[]
> + prb_resp = kmalloc(BCN_TMPL_LEN, GFP_ATOMIC);
> + if (!prb_resp) {
> + wiphy_err(wlc->wiphy, "wl: %s: failed to alloc %u bytes\n",
> + __func__, BCN_TMPL_LEN);
Please remove the error message.
alloc's don't need specific OOM messages.
The mm subsystem already provides a standardized
message with a dump_stack().
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH wireless-next] brcmsmac: avoid 512 byte stack variable
2013-02-07 20:13 [PATCH wireless-next] brcmsmac: avoid 512 byte stack variable Tim Gardner
2013-02-07 20:19 ` Joe Perches
@ 2013-02-07 20:22 ` Arend van Spriel
2013-02-07 20:28 ` [PATCH wireless-next V2] " Tim Gardner
1 sibling, 1 reply; 5+ messages in thread
From: Arend van Spriel @ 2013-02-07 20:22 UTC (permalink / raw)
To: Tim Gardner
Cc: linux-kernel, Brett Rudley, Franky (Zhenhui) Lin, Hante Meuleman,
John W. Linville, Seth Forshee, Pieter-Paul Giesberts,
Hauke Mehrtens, linux-wireless, brcm80211-dev-list, netdev,
Joe Perches
On 02/07/2013 09:13 PM, Tim Gardner wrote:
> Dynamically allocate the probe response template which
> avoids potential stack corruption. Observed with smatch:
>
> drivers/net/wireless/brcm80211/brcmsmac/main.c:7412 brcms_c_bss_update_probe_resp()
> warn: 'prb_resp' puts 512 bytes on stack
>
> Cc: Brett Rudley <brudley@broadcom.com>
> Cc: Arend van Spriel <arend@broadcom.com>
> Cc: "Franky (Zhenhui) Lin" <frankyl@broadcom.com>
> Cc: Hante Meuleman <meuleman@broadcom.com>
> Cc: "John W. Linville" <linville@tuxdriver.com>
> Cc: Seth Forshee <seth.forshee@canonical.com>
> Cc: Pieter-Paul Giesberts <pieterpg@broadcom.com>
> Cc: Hauke Mehrtens <hauke@hauke-m.de>
> Cc: linux-wireless@vger.kernel.org
> Cc: brcm80211-dev-list@broadcom.com
> Cc: netdev@vger.kernel.org
One comment below. When taken care of feel free to add:
Acked-by: Arend van Spriel <arend@broadcom.com>
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
> ---
> drivers/net/wireless/brcm80211/brcmsmac/main.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/brcm80211/brcmsmac/main.c b/drivers/net/wireless/brcm80211/brcmsmac/main.c
> index c26992a..e392e76 100644
> --- a/drivers/net/wireless/brcm80211/brcmsmac/main.c
> +++ b/drivers/net/wireless/brcm80211/brcmsmac/main.c
> @@ -7408,9 +7408,16 @@ brcms_c_bss_update_probe_resp(struct brcms_c_info *wlc,
> struct brcms_bss_cfg *cfg,
> bool suspend)
> {
> - u16 prb_resp[BCN_TMPL_LEN / 2];
> + u16 *prb_resp;
> int len = BCN_TMPL_LEN;
>
> + prb_resp = kmalloc(BCN_TMPL_LEN, GFP_ATOMIC);
> + if (!prb_resp) {
> + wiphy_err(wlc->wiphy, "wl: %s: failed to alloc %u bytes\n",
> + __func__, BCN_TMPL_LEN);
I believe the kmalloc() call spews enough info upon allocation failure
so please remove the error message here.
> + return;
> + }
> +
> /*
> * write the probe response to hardware, or save in
> * the config structure
> @@ -7444,6 +7451,8 @@ brcms_c_bss_update_probe_resp(struct brcms_c_info *wlc,
>
> if (suspend)
> brcms_c_enable_mac(wlc);
> +
> + kfree(prb_resp);
> }
>
> void brcms_c_update_probe_resp(struct brcms_c_info *wlc, bool suspend)
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH wireless-next] brcmsmac: avoid 512 byte stack variable
2013-02-07 20:19 ` Joe Perches
@ 2013-02-07 20:23 ` Arend van Spriel
0 siblings, 0 replies; 5+ messages in thread
From: Arend van Spriel @ 2013-02-07 20:23 UTC (permalink / raw)
To: Joe Perches
Cc: Tim Gardner, linux-kernel, Brett Rudley, Franky (Zhenhui) Lin,
Hante Meuleman, John W. Linville, Seth Forshee,
Pieter-Paul Giesberts, Hauke Mehrtens, linux-wireless,
brcm80211-dev-list, netdev
On 02/07/2013 09:19 PM, Joe Perches wrote:
> On Thu, 2013-02-07 at 13:13 -0700, Tim Gardner wrote:
>> Dynamically allocate the probe response template which
>> avoids potential stack corruption. Observed with smatch:
>
> trivial:
>
>> diff --git a/drivers/net/wireless/brcm80211/brcmsmac/main.c b/drivers/net/wireless/brcm80211/brcmsmac/main.c
> []
>> @@ -7408,9 +7408,16 @@ brcms_c_bss_update_probe_resp(struct brcms_c_info *wlc,
> []
>> + prb_resp = kmalloc(BCN_TMPL_LEN, GFP_ATOMIC);
>> + if (!prb_resp) {
>> + wiphy_err(wlc->wiphy, "wl: %s: failed to alloc %u bytes\n",
>> + __func__, BCN_TMPL_LEN);
>
> Please remove the error message.
> alloc's don't need specific OOM messages.
>
> The mm subsystem already provides a standardized
> message with a dump_stack().
You beat me to the finish line :-)
Gr. AvS
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH wireless-next V2] brcmsmac: avoid 512 byte stack variable
2013-02-07 20:22 ` Arend van Spriel
@ 2013-02-07 20:28 ` Tim Gardner
0 siblings, 0 replies; 5+ messages in thread
From: Tim Gardner @ 2013-02-07 20:28 UTC (permalink / raw)
To: linux-kernel
Cc: Tim Gardner, Brett Rudley, Arend van Spriel,
Franky (Zhenhui) Lin, Hante Meuleman, John W. Linville,
Seth Forshee, Pieter-Paul Giesberts, Hauke Mehrtens,
linux-wireless, brcm80211-dev-list, netdev
Dynamically allocate the probe response template which
avoids potential stack corruption. Observed with smatch:
drivers/net/wireless/brcm80211/brcmsmac/main.c:7412 brcms_c_bss_update_probe_resp()
warn: 'prb_resp' puts 512 bytes on stack
Cc: Brett Rudley <brudley@broadcom.com>
Cc: Arend van Spriel <arend@broadcom.com>
Cc: "Franky (Zhenhui) Lin" <frankyl@broadcom.com>
Cc: Hante Meuleman <meuleman@broadcom.com>
Cc: "John W. Linville" <linville@tuxdriver.com>
Cc: Seth Forshee <seth.forshee@canonical.com>
Cc: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Cc: Hauke Mehrtens <hauke@hauke-m.de>
Cc: linux-wireless@vger.kernel.org
Cc: brcm80211-dev-list@broadcom.com
Cc: netdev@vger.kernel.org
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
drivers/net/wireless/brcm80211/brcmsmac/main.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/brcm80211/brcmsmac/main.c b/drivers/net/wireless/brcm80211/brcmsmac/main.c
index c26992a..ea88abe 100644
--- a/drivers/net/wireless/brcm80211/brcmsmac/main.c
+++ b/drivers/net/wireless/brcm80211/brcmsmac/main.c
@@ -7408,9 +7408,13 @@ brcms_c_bss_update_probe_resp(struct brcms_c_info *wlc,
struct brcms_bss_cfg *cfg,
bool suspend)
{
- u16 prb_resp[BCN_TMPL_LEN / 2];
+ u16 *prb_resp;
int len = BCN_TMPL_LEN;
+ prb_resp = kmalloc(BCN_TMPL_LEN, GFP_ATOMIC);
+ if (!prb_resp)
+ return;
+
/*
* write the probe response to hardware, or save in
* the config structure
@@ -7444,6 +7448,8 @@ brcms_c_bss_update_probe_resp(struct brcms_c_info *wlc,
if (suspend)
brcms_c_enable_mac(wlc);
+
+ kfree(prb_resp);
}
void brcms_c_update_probe_resp(struct brcms_c_info *wlc, bool suspend)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-02-07 20:28 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-07 20:13 [PATCH wireless-next] brcmsmac: avoid 512 byte stack variable Tim Gardner
2013-02-07 20:19 ` Joe Perches
2013-02-07 20:23 ` Arend van Spriel
2013-02-07 20:22 ` Arend van Spriel
2013-02-07 20:28 ` [PATCH wireless-next V2] " Tim Gardner
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).