2012-11-19 16:01:24

by John W. Linville

Subject: [PATCH] brcmfmac: check return from kzalloc in brcmf_fweh_process_event

From: "John W. Linville" <[email protected]>

Signed-off-by: John W. Linville <[email protected]>
Reported-by: Fengguang Wu <[email protected]>
---
drivers/net/wireless/brcm80211/brcmfmac/fweh.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/fweh.c b/drivers/net/wireless/brcm80211/brcmfmac/fweh.c
index 1e4188c..fa8fc44 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/fweh.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/fweh.c
@@ -494,6 +494,9 @@ void brcmf_fweh_process_event(struct brcmf_pub *drvr,
alloc_flag = GFP_ATOMIC;

event = kzalloc(sizeof(*event) + datalen, alloc_flag);
+ if (!event)
+ return;
+
event->code = code;
event->ifidx = *ifidx;

--
1.7.11.7

2012-11-19 17:39:19

by Arend van Spriel

Subject: Re: [PATCH] brcmfmac: check return from kzalloc in brcmf_fweh_process_event

On 11/19/2012 04:53 PM, John W. Linville wrote:
> From: "John W. Linville" <[email protected]>
>
> Signed-off-by: John W. Linville <[email protected]>
> Reported-by: Fengguang Wu <[email protected]>
> ---
> drivers/net/wireless/brcm80211/brcmfmac/fweh.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/fweh.c b/drivers/net/wireless/brcm80211/brcmfmac/fweh.c
> index 1e4188c..fa8fc44 100644
> --- a/drivers/net/wireless/brcm80211/brcmfmac/fweh.c
> +++ b/drivers/net/wireless/brcm80211/brcmfmac/fweh.c
> @@ -494,6 +494,9 @@ void brcmf_fweh_process_event(struct brcmf_pub *drvr,
> alloc_flag = GFP_ATOMIC;
>
> event = kzalloc(sizeof(*event) + datalen, alloc_flag);
> + if (!event)

My fix included a debug statement about the discarded event from firmware.

> + return;
> +
> event->code = code;
> event->ifidx = *ifidx;
>
>

I will deal with the merge so:

Acked-by: Arend van Spriel <[email protected]>

Gr. AvS

2012-11-20 07:57:36

by Kalle Valo

Subject: Re: [PATCH] brcmfmac: check return from kzalloc in brcmf_fweh_process_event

"Arend van Spriel" <[email protected]> writes:

> On 11/19/2012 04:53 PM, John W. Linville wrote:
>> From: "John W. Linville" <[email protected]>
>>
>> Signed-off-by: John W. Linville <[email protected]>
>> Reported-by: Fengguang Wu <[email protected]>

[...]

>> event = kzalloc(sizeof(*event) + datalen, alloc_flag);
>> + if (!event)
>
> My fix included a debug statement about the discarded event from firmware.

Someone once suggested that memory allocation errors should not be
printed by the drivers as the memory subsystem does that anyway. No idea
if that's a good advice or not, but I try to follow it.

--
Kalle Valo

2012-11-20 19:51:47

by Arend van Spriel

Subject: Re: [PATCH] brcmfmac: check return from kzalloc in brcmf_fweh_process_event

On 11/20/2012 08:57 AM, Kalle Valo wrote:
> "Arend van Spriel" <[email protected]> writes:
>
>> On 11/19/2012 04:53 PM, John W. Linville wrote:
>>> From: "John W. Linville" <[email protected]>
>>>
>>> Signed-off-by: John W. Linville <[email protected]>
>>> Reported-by: Fengguang Wu <[email protected]>
>
> [...]
>
>>> event = kzalloc(sizeof(*event) + datalen, alloc_flag);
>>> + if (!event)
>>
>> My fix included a debug statement about the discarded event from firmware.
>
> Someone once suggested that memory allocation errors should not be
> printed by the drivers as the memory subsystem does that anyway. No idea
> if that's a good advice or not, but I try to follow it.
>

Hi Kalle,

In general, I follow the same suggestion. However, in this particular
code path we are handling an event message from the device, which is
being discarded due to the allocation failure. That could be derived
from the trace that kzalloc spews, but I preferred to have it explicitly
stated in the logs.

Gr. AvS