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