[PATCH v4 0/2] wifi: libertas: fix two OOB reads from firmware fields

[PATCH v4 0/2] wifi: libertas: fix two OOB reads from firmware fields

[Tristan Madani]

Two fixes for out-of-bounds reads in libertas caused by unchecked
firmware-supplied fields.

Changes since v3:
  - Use size_add() instead of raw addition/subtraction in bounds checks
    to avoid integer wrapping on 32-bit (Dan Carpenter / kernel test robot)

Tristan Madani (2):
  wifi: libertas: fix OOB read from firmware pkt_ptr offset in RX path
  wifi: libertas: fix OOB read from firmware bssdescriptsize in scan
    response

 drivers/net/wireless/marvell/libertas/cfg.c |  5 +++--
 drivers/net/wireless/marvell/libertas/rx.c  | 10 ++++++++++
 2 files changed, 13 insertions(+), 2 deletions(-)

-- 
2.47.3

[PATCH v4 1/2] wifi: libertas: fix OOB read from firmware pkt_ptr offset in RX path

[Tristan Madani]

lbs_process_rxed_packet() uses the firmware-supplied pkt_ptr as an
offset into the skb data without validating that it falls within the
skb buffer bounds. A malicious pkt_ptr value causes out-of-bounds
memory access when the function subsequently reads ethernet header
fields from p_rx_pkt.

Add a bounds check using size_add() to ensure pkt_ptr plus the minimum
packet header size does not exceed skb->len, without risking integer
overflow on 32-bit platforms.

Fixes: e45d8e534b67 ("libertas: add support for Marvell SD8688 chip")
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <error27@gmail.com>
Closes: https://lore.kernel.org/r/202605030019.lL8x0ZPx-lkp@intel.com/
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
 drivers/net/wireless/marvell/libertas/rx.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/net/wireless/marvell/libertas/rx.c b/drivers/net/wireless/marvell/libertas/rx.c
index c34d30f7cbe03..e6378be4266e1 100644
--- a/drivers/net/wireless/marvell/libertas/rx.c
+++ b/drivers/net/wireless/marvell/libertas/rx.c
@@ -9,6 +9,7 @@
 #include <linux/hardirq.h>
 #include <linux/slab.h>
 #include <linux/types.h>
+#include <linux/overflow.h>
 #include <linux/export.h>
 #include <net/cfg80211.h>
 
@@ -73,6 +74,15 @@ int lbs_process_rxed_packet(struct lbs_private *priv, struct sk_buff *skb)
 	}
 
 	p_rx_pd = (struct rxpd *) skb->data;
+
+	if (size_add(le32_to_cpu(p_rx_pd->pkt_ptr), sizeof(struct rxpackethdr)) >
+	    skb->len) {
+		lbs_deb_rx("rx err: pkt_ptr %u beyond skb len %u\n",
+			   le32_to_cpu(p_rx_pd->pkt_ptr), skb->len);
+		ret = -EINVAL;
+		dev_kfree_skb(skb);
+		goto done;
+	}
 	p_rx_pkt = (struct rxpackethdr *) ((u8 *)p_rx_pd +
 		le32_to_cpu(p_rx_pd->pkt_ptr));
 
-- 
2.47.3

[PATCH v4 2/2] wifi: libertas: fix OOB read from firmware bssdescriptsize in scan response

[Tristan Madani]

The firmware-controlled bssdescriptsize field in lbs_ret_scan() is used
to compute the TSF descriptor position without validation against the
response buffer size. An inflated value causes out-of-bounds reads from
the 2312-byte response buffer into adjacent struct lbs_private members.

Add a check using size_add() that bssdescriptsize plus the response
header size does not exceed the total response size, avoiding integer
wrapping on 32-bit platforms.

Fixes: ff9fc791940f ("libertas: first stab at cfg80211 support")
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
 drivers/net/wireless/marvell/libertas/cfg.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/marvell/libertas/cfg.c b/drivers/net/wireless/marvell/libertas/cfg.c
index 41dee6e0ca9fa..8015adf37c4b0 100644
--- a/drivers/net/wireless/marvell/libertas/cfg.c
+++ b/drivers/net/wireless/marvell/libertas/cfg.c
@@ -13,6 +13,7 @@
 #include <linux/sched.h>
 #include <linux/wait.h>
 #include <linux/slab.h>
+#include <linux/overflow.h>
 #include <linux/ieee80211.h>
 #include <net/cfg80211.h>
 #include <linux/unaligned.h>
@@ -554,8 +555,8 @@ static int lbs_ret_scan(struct lbs_private *priv, unsigned long dummy,
 
 	bsssize = get_unaligned_le16(&scanresp->bssdescriptsize);
 
-	if (bsssize > le16_to_cpu(resp->size) -
-	    sizeof(struct cmd_ds_802_11_scan_rsp)) {
+	if (size_add(bsssize, sizeof(struct cmd_ds_802_11_scan_rsp)) >
+	    le16_to_cpu(resp->size)) {
 		lbs_deb_scan(
 			"scan response: bssdescriptsize %d exceeds response\n",
 			bsssize);
-- 
2.47.3

Re: [PATCH v4 0/2] wifi: libertas: fix two OOB reads from firmware fields

[Johannes Berg]

Tristan,

This is going to be the last time I reply to you if you continue this.

Please:

 1) Send patches that actually apply to the relevant tree
    (wireless or wireless-next in this case, ideally tagging it in the
    subject such as "[PATCH wireless v5 1/2]")

 2) Consider feedback you get. Even in another driver it's not useful to
    check one size against another size if both are provided by the
    device, unless the other is already validated. If so, add a comment.

 3) Disclose AI/LLM usage per
    https://docs.kernel.org/process/coding-assistants.html
    (Yes, I don't believe you came out of nowhere without it.)

johannes