2012-02-22 07:52:57

by Dan Carpenter

[permalink] [raw]
Subject: Re: [patch] Staging: wlan-ng: memsetting the wrong amount of data

On Tue, Feb 21, 2012 at 05:39:42PM +0100, walter harms wrote:
> > - memset(&(msg1.bssid.data), 0xFF, sizeof(p80211item_pstr6_t));
> > + memset(&msg1.bssid.data, 0xFF, sizeof(msg1.bssid.data));
> > msg1.bssid.data.len = 6;
>
> maybe msg1.bssid.data.len is related to msg1.bssid.data ?
> I guess sizeof(msg1.bssid.data)-1 (why -1).
>
> perhaps you can fix both ?
>

It's an interesting point. The problem is that I don't actually
have this hardware. On the patch which I sent, it was obvious what
the intent. My guess is that msg1.bssid.data[] should have 6
elements instead of 7, but I don't feel confident enough to sign off
on that.

Let's fix this bug which is obvious and let someone who knows how to
fix that other question address it.

regards,
dan carpenter


Attachments:
(No filename) (772.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2012-02-22 08:16:59

by walter harms

[permalink] [raw]
Subject: Re: [patch] Staging: wlan-ng: memsetting the wrong amount of data



Am 22.02.2012 08:54, schrieb Dan Carpenter:
> On Tue, Feb 21, 2012 at 05:39:42PM +0100, walter harms wrote:
>>> - memset(&(msg1.bssid.data), 0xFF, sizeof(p80211item_pstr6_t));
>>> + memset(&msg1.bssid.data, 0xFF, sizeof(msg1.bssid.data));
>>> msg1.bssid.data.len = 6;
>>
>> maybe msg1.bssid.data.len is related to msg1.bssid.data ?
>> I guess sizeof(msg1.bssid.data)-1 (why -1).
>>
>> perhaps you can fix both ?
>>
>
> It's an interesting point. The problem is that I don't actually
> have this hardware. On the patch which I sent, it was obvious what
> the intent. My guess is that msg1.bssid.data[] should have 6
> elements instead of 7, but I don't feel confident enough to sign off
> on that.
>
> Let's fix this bug which is obvious and let someone who knows how to
> fix that other question address it.
>

Now it lokks better than before, lets wait what the maintainer can say about this.
otherwise what about a /* FIXME: */ ?

re,
wh

2012-02-23 22:55:30

by Pavel Roskin

[permalink] [raw]
Subject: Re: [patch] Staging: wlan-ng: memsetting the wrong amount of data

On Wed, 22 Feb 2012 09:08:25 +0100
walter harms <[email protected]> wrote:

> Am 22.02.2012 08:54, schrieb Dan Carpenter:
> > On Tue, Feb 21, 2012 at 05:39:42PM +0100, walter harms wrote:
> >>> - memset(&(msg1.bssid.data), 0xFF,
> >>> sizeof(p80211item_pstr6_t));
> >>> + memset(&msg1.bssid.data, 0xFF, sizeof(msg1.bssid.data));
> >>> msg1.bssid.data.len = 6;
> >>
> >> maybe msg1.bssid.data.len is related to msg1.bssid.data ?
> >> I guess sizeof(msg1.bssid.data)-1 (why -1).
> >>
> >> perhaps you can fix both ?
> >>
> >
> > It's an interesting point. The problem is that I don't actually
> > have this hardware. On the patch which I sent, it was obvious what
> > the intent. My guess is that msg1.bssid.data[] should have 6
> > elements instead of 7, but I don't feel confident enough to sign off
> > on that.

msg1.bssid.data.data has 6 elements. msg1.bssid.data is a Pascal
string, i.e. a length byte and 6 bytes of data.

The intention of the code must have been:

memset(&msg1.bssid.data.data, 0xFF, sizeof(msg1.bssid.data.data));

sizeof(msg1.bssid.data.data) is 6.

Writing 15 bytes to a structure that is 7 bytes long is certainly
wrong and should be fixed.

I have the hardware, so please copy me if testing is needed.

--
Regards,
Pavel Roskin