diff mbox

[Trusty] brcmfmac: fix possible buffer overflow in brcmf_cfg80211_mgmt_tx()

Message ID 1503392896-15385-3-git-send-email-stefan.bader@canonical.com
State New
Headers show

Commit Message

Stefan Bader Aug. 22, 2017, 9:08 a.m. UTC 
From: Arend van Spriel <arend.vanspriel@broadcom.com>

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>
Signed-off-by: David S. Miller <davem@davemloft.net>

CVE-2017-7541

(backported from commit 8f44c9a41386729fea410e688959ddaa9d51be7c)
[smb: adjusted path]
Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
---
 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Colin Ian King Aug. 23, 2017, 10:30 a.m. UTC | #1 
On 22/08/17 10:08, Stefan Bader wrote:
> From: Arend van Spriel <arend.vanspriel@broadcom.com>
> 
> 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>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> 
> CVE-2017-7541
> 
> (backported from commit 8f44c9a41386729fea410e688959ddaa9d51be7c)
> [smb: adjusted path]
> Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
> ---
>  drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
> index f2e245b..c61ed8c 100644
> --- a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
> +++ b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
> @@ -4026,6 +4026,11 @@ brcmf_cfg80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
>  		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");
> 
Acked-by: Colin Ian King <colin.king@canonical.com>
Kleber Sacilotto de Souza Aug. 23, 2017, 1:51 p.m. UTC | #2 
On 08/22/17 11:08, Stefan Bader wrote:
> From: Arend van Spriel <arend.vanspriel@broadcom.com>
> 
> 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>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> 
> CVE-2017-7541
> 
> (backported from commit 8f44c9a41386729fea410e688959ddaa9d51be7c)
> [smb: adjusted path]
> Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
> ---
>  drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
> index f2e245b..c61ed8c 100644
> --- a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
> +++ b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
> @@ -4026,6 +4026,11 @@ brcmf_cfg80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
>  		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");
> 

Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>
Kleber Sacilotto de Souza Aug. 23, 2017, 2:45 p.m. UTC | #3 
Applied to trusty/master-next branch. Thanks.
diff mbox

Patch

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
index f2e245b..c61ed8c 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
@@ -4026,6 +4026,11 @@  brcmf_cfg80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
 		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");