linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V2] brcmfmac: fix possible buffer overflow in brcmf_cfg80211_mgmt_tx()
@ 2017-07-07 20:09 Arend van Spriel
  2017-07-07 20:11 ` Linus Torvalds
  2017-07-12 11:49 ` Arend van Spriel
  0 siblings, 2 replies; 4+ messages in thread
From: Arend van Spriel @ 2017-07-07 20:09 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-wireless, Linus Torvalds, Arend van Spriel

The lower level nl80211 code in cfg80211 ensures that "len" is between
25 and NL80211_ATTR_FRAME (2304).  We subtract DOT11_MGMT_HDR_LEN (24) from
"len" so thats's max of 2280.  However, the action_frame->data[] buffer is
only BRCMF_FIL_ACTION_FRAME_SIZE (1800) bytes long so this memcpy() can
overflow.

	memcpy(action_frame->data, &buf[DOT11_MGMT_HDR_LEN],
	       le16_to_cpu(action_frame->len));

Cc: stable@vger.kernel.org # 3.9.x
Fixes: 18e2f61db3b70 ("brcmfmac: P2P action frame tx.")
Reported-by: "freenerguo(郭大兴)" <freenerguo@tencent.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 V2:
  - added Fixes: tag and Cc: for stable kernels.
  - Cc: patch to netdev list.
---
Hi David,

Here is the patch as Linus send it to us and security@kernel.org. I
removed the lower bound check as that is already done in cfg80211.
Now I signed off on the patch although formally I suppose Linus should
sign it off. Putting it out there so people can respond as deemed
necessary.

The reason for submitting it to your tree is the fact that Kalle is
on vacation for next 10 days or so which was indicated to me by Johannes.
The patch applies to the master branch of your net repository. For
reference V1 of this patch can be found here [1].

Regards,
Arend

[1] https://patchwork.kernel.org/patch/9829977/
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index cd1d673..d182a00 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -4851,6 +4851,11 @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev)
 		cfg80211_mgmt_tx_status(wdev, *cookie, buf, len, true,
 					GFP_KERNEL);
 	} else if (ieee80211_is_action(mgmt->frame_control)) {
+		if (len > BRCMF_FIL_ACTION_FRAME_SIZE + DOT11_MGMT_HDR_LEN) {
+			brcmf_err("invalid action frame length\n");
+			err = -EINVAL;
+			goto exit;
+		}
 		af_params = kzalloc(sizeof(*af_params), GFP_KERNEL);
 		if (af_params == NULL) {
 			brcmf_err("unable to allocate frame\n");
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH V2] brcmfmac: fix possible buffer overflow in brcmf_cfg80211_mgmt_tx()
  2017-07-07 20:09 [PATCH V2] brcmfmac: fix possible buffer overflow in brcmf_cfg80211_mgmt_tx() Arend van Spriel
@ 2017-07-07 20:11 ` Linus Torvalds
  2017-07-12 11:49 ` Arend van Spriel
  1 sibling, 0 replies; 4+ messages in thread
From: Linus Torvalds @ 2017-07-07 20:11 UTC (permalink / raw)
  To: Arend van Spriel; +Cc: David Miller, Network Development, Linux Wireless List

On Fri, Jul 7, 2017 at 1:09 PM, Arend van Spriel
<arend.vanspriel@broadcom.com> wrote:
> Now I signed off on the patch although formally I suppose Linus should
> sign it off.

You can certainly consider it

   Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

but I really don't need the authorship (or resulting sign-off
requirement) because multiple people ended up sending in very similar
patches.

All the real work was in actually finding the issue.

                  Linus

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH V2] brcmfmac: fix possible buffer overflow in brcmf_cfg80211_mgmt_tx()
  2017-07-07 20:09 [PATCH V2] brcmfmac: fix possible buffer overflow in brcmf_cfg80211_mgmt_tx() Arend van Spriel
  2017-07-07 20:11 ` Linus Torvalds
@ 2017-07-12 11:49 ` Arend van Spriel
  2017-07-12 15:28   ` David Miller
  1 sibling, 1 reply; 4+ messages in thread
From: Arend van Spriel @ 2017-07-12 11:49 UTC (permalink / raw)
  To: Arend van Spriel; +Cc: David Miller, netdev, linux-wireless, Linus Torvalds

On 7/7/2017 10:09 PM, Arend van Spriel wrote:
> The lower level nl80211 code in cfg80211 ensures that "len" is between
> 25 and NL80211_ATTR_FRAME (2304).  We subtract DOT11_MGMT_HDR_LEN (24) from
> "len" so thats's max of 2280.  However, the action_frame->data[] buffer is
> only BRCMF_FIL_ACTION_FRAME_SIZE (1800) bytes long so this memcpy() can
> overflow.
>
> 	memcpy(action_frame->data, &buf[DOT11_MGMT_HDR_LEN],
> 	       le16_to_cpu(action_frame->len));
>
> Cc: stable@vger.kernel.org # 3.9.x
> Fixes: 18e2f61db3b70 ("brcmfmac: P2P action frame tx.")
> Reported-by: "freenerguo(郭大兴)" <freenerguo@tencent.com>
> Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> ---
>   V2:
>    - added Fixes: tag and Cc: for stable kernels.
>    - Cc: patch to netdev list.
> ---
> Hi David,
>
> Here is the patch as Linus send it to us and security@kernel.org. I
> removed the lower bound check as that is already done in cfg80211.
> Now I signed off on the patch although formally I suppose Linus should
> sign it off. Putting it out there so people can respond as deemed
> necessary.
>
> The reason for submitting it to your tree is the fact that Kalle is
> on vacation for next 10 days or so which was indicated to me by Johannes.
> The patch applies to the master branch of your net repository. For
> reference V1 of this patch can be found here [1].

Hi Dave,

Not sure if you missed this one. It is addressing a reported security 
issue and intended for the net repository, not net-next which is 
obviously closed [2].

Regards,
Arend

[2] http://vger.kernel.org/~davem/net-next.html

> Regards,
> Arend
>
> [1] https://patchwork.kernel.org/patch/9829977/
> ---
>   drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> index cd1d673..d182a00 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> @@ -4851,6 +4851,11 @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev)
>   		cfg80211_mgmt_tx_status(wdev, *cookie, buf, len, true,
>   					GFP_KERNEL);
>   	} else if (ieee80211_is_action(mgmt->frame_control)) {
> +		if (len > BRCMF_FIL_ACTION_FRAME_SIZE + DOT11_MGMT_HDR_LEN) {
> +			brcmf_err("invalid action frame length\n");
> +			err = -EINVAL;
> +			goto exit;
> +		}
>   		af_params = kzalloc(sizeof(*af_params), GFP_KERNEL);
>   		if (af_params == NULL) {
>   			brcmf_err("unable to allocate frame\n");
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH V2] brcmfmac: fix possible buffer overflow in brcmf_cfg80211_mgmt_tx()
  2017-07-12 11:49 ` Arend van Spriel
@ 2017-07-12 15:28   ` David Miller
  0 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2017-07-12 15:28 UTC (permalink / raw)
  To: arend.vanspriel; +Cc: netdev, linux-wireless, torvalds

RnJvbTogQXJlbmQgdmFuIFNwcmllbCA8YXJlbmQudmFuc3ByaWVsQGJyb2FkY29tLmNvbT4NCkRh
dGU6IFdlZCwgMTIgSnVsIDIwMTcgMTM6NDk6MjMgKzAyMDANCg0KPiBPbiA3LzcvMjAxNyAxMDow
OSBQTSwgQXJlbmQgdmFuIFNwcmllbCB3cm90ZToNCj4+IFRoZSBsb3dlciBsZXZlbCBubDgwMjEx
IGNvZGUgaW4gY2ZnODAyMTEgZW5zdXJlcyB0aGF0ICJsZW4iIGlzIGJldHdlZW4NCj4+IDI1IGFu
ZCBOTDgwMjExX0FUVFJfRlJBTUUgKDIzMDQpLiAgV2Ugc3VidHJhY3QgRE9UMTFfTUdNVF9IRFJf
TEVOICgyNCkNCj4+IGZyb20NCj4+ICJsZW4iIHNvIHRoYXRzJ3MgbWF4IG9mIDIyODAuICBIb3dl
dmVyLCB0aGUgYWN0aW9uX2ZyYW1lLT5kYXRhW10NCj4+IGJ1ZmZlciBpcw0KPj4gb25seSBCUkNN
Rl9GSUxfQUNUSU9OX0ZSQU1FX1NJWkUgKDE4MDApIGJ5dGVzIGxvbmcgc28gdGhpcyBtZW1jcHko
KQ0KPj4gY2FuDQo+PiBvdmVyZmxvdy4NCj4+DQo+PiAJbWVtY3B5KGFjdGlvbl9mcmFtZS0+ZGF0
YSwgJmJ1ZltET1QxMV9NR01UX0hEUl9MRU5dLA0KPj4gCSAgICAgICBsZTE2X3RvX2NwdShhY3Rp
b25fZnJhbWUtPmxlbikpOw0KPj4NCj4+IENjOiBzdGFibGVAdmdlci5rZXJuZWwub3JnICMgMy45
LngNCj4+IEZpeGVzOiAxOGUyZjYxZGIzYjcwICgiYnJjbWZtYWM6IFAyUCBhY3Rpb24gZnJhbWUg
dHguIikNCj4+IFJlcG9ydGVkLWJ5OiAiZnJlZW5lcmd1byjpg63lpKflhbQpIiA8ZnJlZW5lcmd1
b0B0ZW5jZW50LmNvbT4NCj4+IFNpZ25lZC1vZmYtYnk6IEFyZW5kIHZhbiBTcHJpZWwgPGFyZW5k
LnZhbnNwcmllbEBicm9hZGNvbS5jb20+DQo+PiAtLS0NCj4+ICAgVjI6DQo+PiAgICAtIGFkZGVk
IEZpeGVzOiB0YWcgYW5kIENjOiBmb3Igc3RhYmxlIGtlcm5lbHMuDQo+PiAgICAtIENjOiBwYXRj
aCB0byBuZXRkZXYgbGlzdC4NCj4+IC0tLQ0KPj4gSGkgRGF2aWQsDQo+Pg0KPj4gSGVyZSBpcyB0
aGUgcGF0Y2ggYXMgTGludXMgc2VuZCBpdCB0byB1cyBhbmQgc2VjdXJpdHlAa2VybmVsLm9yZy4g
SQ0KPj4gcmVtb3ZlZCB0aGUgbG93ZXIgYm91bmQgY2hlY2sgYXMgdGhhdCBpcyBhbHJlYWR5IGRv
bmUgaW4gY2ZnODAyMTEuDQo+PiBOb3cgSSBzaWduZWQgb2ZmIG9uIHRoZSBwYXRjaCBhbHRob3Vn
aCBmb3JtYWxseSBJIHN1cHBvc2UgTGludXMgc2hvdWxkDQo+PiBzaWduIGl0IG9mZi4gUHV0dGlu
ZyBpdCBvdXQgdGhlcmUgc28gcGVvcGxlIGNhbiByZXNwb25kIGFzIGRlZW1lZA0KPj4gbmVjZXNz
YXJ5Lg0KPj4NCj4+IFRoZSByZWFzb24gZm9yIHN1Ym1pdHRpbmcgaXQgdG8geW91ciB0cmVlIGlz
IHRoZSBmYWN0IHRoYXQgS2FsbGUgaXMNCj4+IG9uIHZhY2F0aW9uIGZvciBuZXh0IDEwIGRheXMg
b3Igc28gd2hpY2ggd2FzIGluZGljYXRlZCB0byBtZSBieQ0KPj4gSm9oYW5uZXMuDQo+PiBUaGUg
cGF0Y2ggYXBwbGllcyB0byB0aGUgbWFzdGVyIGJyYW5jaCBvZiB5b3VyIG5ldCByZXBvc2l0b3J5
LiBGb3INCj4+IHJlZmVyZW5jZSBWMSBvZiB0aGlzIHBhdGNoIGNhbiBiZSBmb3VuZCBoZXJlIFsx
XS4NCj4gDQo+IEhpIERhdmUsDQo+IA0KPiBOb3Qgc3VyZSBpZiB5b3UgbWlzc2VkIHRoaXMgb25l
LiBJdCBpcyBhZGRyZXNzaW5nIGEgcmVwb3J0ZWQgc2VjdXJpdHkNCj4gaXNzdWUgYW5kIGludGVu
ZGVkIGZvciB0aGUgbmV0IHJlcG9zaXRvcnksIG5vdCBuZXQtbmV4dCB3aGljaCBpcw0KPiBvYnZp
b3VzbHkgY2xvc2VkIFsyXS4NCg0KVGhhbmtzIGZvciBwb2ludGluZyB0aGlzIG91dCB0byBtZSwg
SSdsbCB0YWtlIGNhcmUgb2YgaXQuDQoNCg==

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-07-12 15:28 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-07 20:09 [PATCH V2] brcmfmac: fix possible buffer overflow in brcmf_cfg80211_mgmt_tx() Arend van Spriel
2017-07-07 20:11 ` Linus Torvalds
2017-07-12 11:49 ` Arend van Spriel
2017-07-12 15:28   ` David Miller

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).