* [patch 2/3] brcmfmac: fix end of loop check (signedness bug)
@ 2012-10-03 6:06 Dan Carpenter
2012-10-04 16:24 ` Arend van Spriel
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2012-10-03 6:06 UTC (permalink / raw)
To: Brett Rudley
Cc: Roland Vossen, Arend van Spriel, Franky (Zhenhui) Lin, Kan Yan,
John W. Linville, Hante Meuleman, Pieter-Paul Giesberts,
linux-wireless, brcm80211-dev-list, kernel-janitors
The problem here is that we loop until "remained_buf_len" is less than
zero, but since it is unsigned, it never is.
"remained_buf_len" has to be large enough to hold the value from
"mgmt_ie_buf_len". That variable is type u32, but it only holds small
values so I have changed to both variables to int.
Also I removed the bogus initialization from "mgmt_ie_buf_len" so that
GCC can detect if it is used unitialized. I moved the declaration of
"remained_buf_len" closer to where it is used so it's easier to read.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
index c1abaa6..ed83b54 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
@@ -3972,7 +3972,7 @@ brcmf_set_management_ie(struct brcmf_cfg80211_info *cfg,
u8 *iovar_ie_buf;
u8 *curr_ie_buf;
u8 *mgmt_ie_buf = NULL;
- u32 mgmt_ie_buf_len = 0;
+ int mgmt_ie_buf_len;
u32 *mgmt_ie_len = 0;
u32 del_add_ie_buf_len = 0;
u32 total_ie_buf_len = 0;
@@ -3982,7 +3982,6 @@ brcmf_set_management_ie(struct brcmf_cfg80211_info *cfg,
struct parsed_vndr_ie_info *vndrie_info;
s32 i;
u8 *ptr;
- u32 remained_buf_len;
WL_TRACE("bssidx %d, pktflag : 0x%02X\n", bssidx, pktflag);
iovar_ie_buf = kzalloc(WL_EXTRA_BUF_MAX, GFP_KERNEL);
@@ -3995,8 +3994,7 @@ brcmf_set_management_ie(struct brcmf_cfg80211_info *cfg,
case VNDR_IE_PRBRSP_FLAG:
mgmt_ie_buf = cfg->ap_info->probe_res_ie;
mgmt_ie_len = &cfg->ap_info->probe_res_ie_len;
- mgmt_ie_buf_len =
- sizeof(cfg->ap_info->probe_res_ie);
+ mgmt_ie_buf_len = sizeof(cfg->ap_info->probe_res_ie);
break;
case VNDR_IE_BEACON_FLAG:
mgmt_ie_buf = cfg->ap_info->beacon_ie;
@@ -4067,8 +4065,9 @@ brcmf_set_management_ie(struct brcmf_cfg80211_info *cfg,
*mgmt_ie_len = 0;
/* Add if there is any extra IE */
if (mgmt_ie_buf && parsed_ie_buf_len) {
- ptr = mgmt_ie_buf;
+ int remained_buf_len;
+ ptr = mgmt_ie_buf;
remained_buf_len = mgmt_ie_buf_len;
/* make a command to add new ie */
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [patch 2/3] brcmfmac: fix end of loop check (signedness bug)
2012-10-03 6:06 [patch 2/3] brcmfmac: fix end of loop check (signedness bug) Dan Carpenter
@ 2012-10-04 16:24 ` Arend van Spriel
2012-10-04 18:42 ` Dan Carpenter
0 siblings, 1 reply; 4+ messages in thread
From: Arend van Spriel @ 2012-10-04 16:24 UTC (permalink / raw)
To: Dan Carpenter
Cc: Brett Rudley, Roland Vossen, Franky (Zhenhui) Lin, Kan Yan,
John W. Linville, Hante Meuleman, Pieter-Paul Giesberts,
linux-wireless, brcm80211-dev-list, kernel-janitors
On 10/03/2012 08:06 AM, Dan Carpenter wrote:
> The problem here is that we loop until "remained_buf_len" is less than
> zero, but since it is unsigned, it never is.
>
> "remained_buf_len" has to be large enough to hold the value from
> "mgmt_ie_buf_len". That variable is type u32, but it only holds small
> values so I have changed to both variables to int.
>
> Also I removed the bogus initialization from "mgmt_ie_buf_len" so that
> GCC can detect if it is used unitialized. I moved the declaration of
> "remained_buf_len" closer to where it is used so it's easier to read.
>
Hi Dan,
Good catch. I applied the patch internally on our HEAD and had it
reviewed. We did not take moving the declaration as we prefer to have
all variables at the top of the function. It makes it easier to find
what is declared in a function and whether exceeding the local variable
limit mentioned in Chapter 6. Functions of the CodingStyle (we are
exceeding it already ;-) ).
Are you ok with us submitting it? It would be sent out for 3.8 or do you
prefer to have it fixed in 3.7?
Gr. AvS
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [patch 2/3] brcmfmac: fix end of loop check (signedness bug)
2012-10-04 16:24 ` Arend van Spriel
@ 2012-10-04 18:42 ` Dan Carpenter
2012-10-04 21:10 ` Arend van Spriel
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2012-10-04 18:42 UTC (permalink / raw)
To: Arend van Spriel
Cc: Brett Rudley, Roland Vossen, Franky (Zhenhui) Lin, Kan Yan,
John W. Linville, Hante Meuleman, Pieter-Paul Giesberts,
linux-wireless, brcm80211-dev-list, kernel-janitors
On Thu, Oct 04, 2012 at 06:24:11PM +0200, Arend van Spriel wrote:
> On 10/03/2012 08:06 AM, Dan Carpenter wrote:
> >The problem here is that we loop until "remained_buf_len" is less than
> >zero, but since it is unsigned, it never is.
> >
> >"remained_buf_len" has to be large enough to hold the value from
> >"mgmt_ie_buf_len". That variable is type u32, but it only holds small
> >values so I have changed to both variables to int.
> >
> >Also I removed the bogus initialization from "mgmt_ie_buf_len" so that
> >GCC can detect if it is used unitialized. I moved the declaration of
> >"remained_buf_len" closer to where it is used so it's easier to read.
> >
>
> Hi Dan,
>
> Good catch. I applied the patch internally on our HEAD and had it
> reviewed. We did not take moving the declaration as we prefer to
> have all variables at the top of the function. It makes it easier to
> find what is declared in a function and whether exceeding the local
> variable limit mentioned in Chapter 6. Functions of the CodingStyle
> (we are exceeding it already ;-) ).
>
> Are you ok with us submitting it? It would be sent out for 3.8 or do
> you prefer to have it fixed in 3.7?
Uh, I don't know how hard it is to trigger this bug. If it's
impossible to trigger then we could wait until 3.8 otherwise I
would tend to merge it into 3.7 given that we haven't hit -rc1 yet.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [patch 2/3] brcmfmac: fix end of loop check (signedness bug)
2012-10-04 18:42 ` Dan Carpenter
@ 2012-10-04 21:10 ` Arend van Spriel
0 siblings, 0 replies; 4+ messages in thread
From: Arend van Spriel @ 2012-10-04 21:10 UTC (permalink / raw)
To: Dan Carpenter
Cc: Brett Rudley, Roland Vossen, Franky (Zhenhui) Lin, Kan Yan,
John W. Linville, Hante Meuleman, Pieter-Paul Giesberts,
linux-wireless, brcm80211-dev-list, kernel-janitors
On 10/04/2012 08:42 PM, Dan Carpenter wrote:
> On Thu, Oct 04, 2012 at 06:24:11PM +0200, Arend van Spriel wrote:
>> On 10/03/2012 08:06 AM, Dan Carpenter wrote:
>>> The problem here is that we loop until "remained_buf_len" is less than
>>> zero, but since it is unsigned, it never is.
>>>
>>> "remained_buf_len" has to be large enough to hold the value from
>>> "mgmt_ie_buf_len". That variable is type u32, but it only holds small
>>> values so I have changed to both variables to int.
>>>
>>> Also I removed the bogus initialization from "mgmt_ie_buf_len" so that
>>> GCC can detect if it is used unitialized. I moved the declaration of
>>> "remained_buf_len" closer to where it is used so it's easier to read.
>>>
>>
>> Hi Dan,
>>
>> Good catch. I applied the patch internally on our HEAD and had it
>> reviewed. We did not take moving the declaration as we prefer to
>> have all variables at the top of the function. It makes it easier to
>> find what is declared in a function and whether exceeding the local
>> variable limit mentioned in Chapter 6. Functions of the CodingStyle
>> (we are exceeding it already ;-) ).
>>
>> Are you ok with us submitting it? It would be sent out for 3.8 or do
>> you prefer to have it fixed in 3.7?
>
> Uh, I don't know how hard it is to trigger this bug. If it's
> impossible to trigger then we could wait until 3.8 otherwise I
> would tend to merge it into 3.7 given that we haven't hit -rc1 yet.
Hi Dan,
I don't know either, but to safe myself the trouble I will post it to
John for 3.7. Thanks.
Gr. AvS
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-10-04 21:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-03 6:06 [patch 2/3] brcmfmac: fix end of loop check (signedness bug) Dan Carpenter
2012-10-04 16:24 ` Arend van Spriel
2012-10-04 18:42 ` Dan Carpenter
2012-10-04 21:10 ` Arend van Spriel
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).