Takashi Iwai 73cf35
From 820aa37638a252b57967bdf4038a514b1ab85d45 Mon Sep 17 00:00:00 2001
Takashi Iwai 73cf35
From: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Takashi Iwai 73cf35
Date: Wed, 14 Apr 2021 18:43:19 -0500
Takashi Iwai 73cf35
Subject: [PATCH] wl3501_cs: Fix out-of-bounds warnings in wl3501_send_pkt
Takashi Iwai 73cf35
Git-commit: 820aa37638a252b57967bdf4038a514b1ab85d45
Takashi Iwai 73cf35
Patch-mainline: v5.13-rc1
Takashi Iwai 73cf35
References: git-fixes
Takashi Iwai 73cf35
Takashi Iwai 73cf35
Fix the following out-of-bounds warnings by enclosing structure members
Takashi Iwai 73cf35
daddr and saddr into new struct addr, in structures wl3501_md_req and
Takashi Iwai 73cf35
Wl3501_md_ind: 
Takashi Iwai 73cf35
Takashi Iwai 73cf35
arch/x86/include/asm/string_32.h:182:25: warning: '__builtin_memcpy' offset [18, 23] from the object at 'sig' is out of the bounds of referenced subobject 'daddr' with type 'u8[6]' {aka 'unsigned char[6]'} at offset 11 [-Warray-bounds]
Takashi Iwai 73cf35
arch/x86/include/asm/string_32.h:182:25: warning: '__builtin_memcpy' offset [18, 23] from the object at 'sig' is out of the bounds of referenced subobject 'daddr' with type 'u8[6]' {aka 'unsigned char[6]'} at offset 11 [-Warray-bounds]
Takashi Iwai 73cf35
Takashi Iwai 73cf35
Refactor the code, accordingly:
Takashi Iwai 73cf35
Takashi Iwai 73cf35
$ pahole -C wl3501_md_req drivers/net/wireless/wl3501_cs.o
Takashi Iwai 73cf35
struct wl3501_md_req {
Takashi Iwai 73cf35
	u16                        next_blk;             /*     0     2 */
Takashi Iwai 73cf35
	u8                         sig_id;               /*     2     1 */
Takashi Iwai 73cf35
	u8                         routing;              /*     3     1 */
Takashi Iwai 73cf35
	u16                        data;                 /*     4     2 */
Takashi Iwai 73cf35
	u16                        size;                 /*     6     2 */
Takashi Iwai 73cf35
	u8                         pri;                  /*     8     1 */
Takashi Iwai 73cf35
	u8                         service_class;        /*     9     1 */
Takashi Iwai 73cf35
	struct {
Takashi Iwai 73cf35
		u8                 daddr[6];             /*    10     6 */
Takashi Iwai 73cf35
		u8                 saddr[6];             /*    16     6 */
Takashi Iwai 73cf35
	} addr;                                          /*    10    12 */
Takashi Iwai 73cf35
Takashi Iwai 73cf35
	/* size: 22, cachelines: 1, members: 8 */
Takashi Iwai 73cf35
	/* last cacheline: 22 bytes */
Takashi Iwai 73cf35
};
Takashi Iwai 73cf35
Takashi Iwai 73cf35
$ pahole -C wl3501_md_ind drivers/net/wireless/wl3501_cs.o
Takashi Iwai 73cf35
struct wl3501_md_ind {
Takashi Iwai 73cf35
	u16                        next_blk;             /*     0     2 */
Takashi Iwai 73cf35
	u8                         sig_id;               /*     2     1 */
Takashi Iwai 73cf35
	u8                         routing;              /*     3     1 */
Takashi Iwai 73cf35
	u16                        data;                 /*     4     2 */
Takashi Iwai 73cf35
	u16                        size;                 /*     6     2 */
Takashi Iwai 73cf35
	u8                         reception;            /*     8     1 */
Takashi Iwai 73cf35
	u8                         pri;                  /*     9     1 */
Takashi Iwai 73cf35
	u8                         service_class;        /*    10     1 */
Takashi Iwai 73cf35
	struct {
Takashi Iwai 73cf35
		u8                 daddr[6];             /*    11     6 */
Takashi Iwai 73cf35
		u8                 saddr[6];             /*    17     6 */
Takashi Iwai 73cf35
	} addr;                                          /*    11    12 */
Takashi Iwai 73cf35
Takashi Iwai 73cf35
	/* size: 24, cachelines: 1, members: 9 */
Takashi Iwai 73cf35
	/* padding: 1 */
Takashi Iwai 73cf35
	/* last cacheline: 24 bytes */
Takashi Iwai 73cf35
};
Takashi Iwai 73cf35
Takashi Iwai 73cf35
The problem is that the original code is trying to copy data into a
Takashi Iwai 73cf35
couple of arrays adjacent to each other in a single call to memcpy().
Takashi Iwai 73cf35
Now that a new struct _addr_ enclosing those two adjacent arrays
Takashi Iwai 73cf35
is introduced, memcpy() doesn't overrun the length of &sig.daddr[0]
Takashi Iwai 73cf35
and &sig.daddr, because the address of the new struct object _addr_
Takashi Iwai 73cf35
is used, instead.
Takashi Iwai 73cf35
Takashi Iwai 73cf35
This helps with the ongoing efforts to globally enable -Warray-bounds
Takashi Iwai 73cf35
and get us closer to being able to tighten the FORTIFY_SOURCE routines
Takashi Iwai 73cf35
on memcpy().
Takashi Iwai 73cf35
Takashi Iwai 73cf35
Link: https://github.com/KSPP/linux/issues/109
Takashi Iwai 73cf35
Reported-by: kernel test robot <lkp@intel.com>
Takashi Iwai 73cf35
Reviewed-by: Kees Cook <keescook@chromium.org>
Takashi Iwai 73cf35
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Takashi Iwai 73cf35
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Takashi Iwai 73cf35
Link: https://lore.kernel.org/r/d260fe56aed7112bff2be5b4d152d03ad7b78e78.1618442265.git.gustavoars@kernel.org
Takashi Iwai 73cf35
Acked-by: Takashi Iwai <tiwai@suse.de>
Takashi Iwai 73cf35
Takashi Iwai 73cf35
---
Takashi Iwai 73cf35
 drivers/net/wireless/wl3501.h    | 12 ++++++++----
Takashi Iwai 73cf35
 drivers/net/wireless/wl3501_cs.c | 10 ++++++----
Takashi Iwai 73cf35
 2 files changed, 14 insertions(+), 8 deletions(-)
Takashi Iwai 73cf35
Takashi Iwai 73cf35
diff --git a/drivers/net/wireless/wl3501.h b/drivers/net/wireless/wl3501.h
Takashi Iwai 73cf35
index 5779ffbe5d0f..8c5480bec104 100644
Takashi Iwai 73cf35
--- a/drivers/net/wireless/wl3501.h
Takashi Iwai 73cf35
+++ b/drivers/net/wireless/wl3501.h
Takashi Iwai 73cf35
@@ -471,8 +471,10 @@ struct wl3501_md_req {
Takashi Iwai 73cf35
 	u16	size;
Takashi Iwai 73cf35
 	u8	pri;
Takashi Iwai 73cf35
 	u8	service_class;
Takashi Iwai 73cf35
-	u8	daddr[ETH_ALEN];
Takashi Iwai 73cf35
-	u8	saddr[ETH_ALEN];
Takashi Iwai 73cf35
+	struct {
Takashi Iwai 73cf35
+		u8	daddr[ETH_ALEN];
Takashi Iwai 73cf35
+		u8	saddr[ETH_ALEN];
Takashi Iwai 73cf35
+	} addr;
Takashi Iwai 73cf35
 };
Takashi Iwai 73cf35
 
Takashi Iwai 73cf35
 struct wl3501_md_ind {
Takashi Iwai 73cf35
@@ -484,8 +486,10 @@ struct wl3501_md_ind {
Takashi Iwai 73cf35
 	u8	reception;
Takashi Iwai 73cf35
 	u8	pri;
Takashi Iwai 73cf35
 	u8	service_class;
Takashi Iwai 73cf35
-	u8	daddr[ETH_ALEN];
Takashi Iwai 73cf35
-	u8	saddr[ETH_ALEN];
Takashi Iwai 73cf35
+	struct {
Takashi Iwai 73cf35
+		u8	daddr[ETH_ALEN];
Takashi Iwai 73cf35
+		u8	saddr[ETH_ALEN];
Takashi Iwai 73cf35
+	} addr;
Takashi Iwai 73cf35
 };
Takashi Iwai 73cf35
 
Takashi Iwai 73cf35
 struct wl3501_md_confirm {
Takashi Iwai 73cf35
diff --git a/drivers/net/wireless/wl3501_cs.c b/drivers/net/wireless/wl3501_cs.c
Takashi Iwai 73cf35
index 8ca5789c7b37..70307308635f 100644
Takashi Iwai 73cf35
--- a/drivers/net/wireless/wl3501_cs.c
Takashi Iwai 73cf35
+++ b/drivers/net/wireless/wl3501_cs.c
Takashi Iwai 73cf35
@@ -469,6 +469,7 @@ static int wl3501_send_pkt(struct wl3501_card *this, u8 *data, u16 len)
Takashi Iwai 73cf35
 	struct wl3501_md_req sig = {
Takashi Iwai 73cf35
 		.sig_id = WL3501_SIG_MD_REQ,
Takashi Iwai 73cf35
 	};
Takashi Iwai 73cf35
+	size_t sig_addr_len = sizeof(sig.addr);
Takashi Iwai 73cf35
 	u8 *pdata = (char *)data;
Takashi Iwai 73cf35
 	int rc = -EIO;
Takashi Iwai 73cf35
 
Takashi Iwai 73cf35
@@ -484,9 +485,9 @@ static int wl3501_send_pkt(struct wl3501_card *this, u8 *data, u16 len)
Takashi Iwai 73cf35
 			goto out;
Takashi Iwai 73cf35
 		}
Takashi Iwai 73cf35
 		rc = 0;
Takashi Iwai 73cf35
-		memcpy(&sig.daddr[0], pdata, 12);
Takashi Iwai 73cf35
-		pktlen = len - 12;
Takashi Iwai 73cf35
-		pdata += 12;
Takashi Iwai 73cf35
+		memcpy(&sig.addr, pdata, sig_addr_len);
Takashi Iwai 73cf35
+		pktlen = len - sig_addr_len;
Takashi Iwai 73cf35
+		pdata += sig_addr_len;
Takashi Iwai 73cf35
 		sig.data = bf;
Takashi Iwai 73cf35
 		if (((*pdata) * 256 + (*(pdata + 1))) > 1500) {
Takashi Iwai 73cf35
 			u8 addr4[ETH_ALEN] = {
Takashi Iwai 73cf35
@@ -980,7 +981,8 @@ static inline void wl3501_md_ind_interrupt(struct net_device *dev,
Takashi Iwai 73cf35
 	} else {
Takashi Iwai 73cf35
 		skb->dev = dev;
Takashi Iwai 73cf35
 		skb_reserve(skb, 2); /* IP headers on 16 bytes boundaries */
Takashi Iwai 73cf35
-		skb_copy_to_linear_data(skb, (unsigned char *)&sig.daddr, 12);
Takashi Iwai 73cf35
+		skb_copy_to_linear_data(skb, (unsigned char *)&sig.addr,
Takashi Iwai 73cf35
+					sizeof(sig.addr));
Takashi Iwai 73cf35
 		wl3501_receive(this, skb->data, pkt_len);
Takashi Iwai 73cf35
 		skb_put(skb, pkt_len);
Takashi Iwai 73cf35
 		skb->protocol	= eth_type_trans(skb, dev);
Takashi Iwai 73cf35
-- 
Takashi Iwai 73cf35
2.26.2
Takashi Iwai 73cf35