[PATCH v2 0/3] wifi: carl9170: fix buffer overflow and OOB reads in firmware response handling

[PATCH v2 0/3] wifi: carl9170: fix buffer overflow and OOB reads in firmware response handling

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

Hi Christian,

Note: this is a v2 resubmission. The original was sent via Gmail which
caused HTML rendering issues. This version uses git send-email for
proper plain-text formatting.

Three issues in carl9170 firmware response handling.

Changes since v1:
- Patch 1/3 (cmd_callback memcpy): bound with min_t() instead of early
  return after carl9170_restart(), per your feedback.

Proposed fixes in the following patches.

Thanks,
Tristan

[PATCH v2 1/3] wifi: carl9170: bound memcpy length in cmd callback to prevent OOB read

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

When the firmware sends a command response with a length mismatch,
carl9170_cmd_callback() logs the mismatch and calls carl9170_restart()
but then falls through to memcpy(ar->readbuf, buffer + 4, len - 4).
Since len comes from the firmware and can exceed ar->readlen, this
copies more data than the readbuf was allocated for.

Bound the memcpy to min(len - 4, ar->readlen) so that the response
is still completed -- avoiding repeated restarts from queued garbage --
while preventing an overread past the response buffer.

Fixes: a84fab3cbfdc ("carl9170: 802.11 rx/tx processing and usb backend")
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
Changes in v2:
  - v2: bound memcpy with min_t() instead of adding early return after
    carl9170_restart(), per Christian Lamparter's feedback. The restart
    path must handle queued responses gracefully.

drivers/net/wireless/ath/carl9170/rx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/carl9170/rx.c b/drivers/net/wireless/ath/carl9170/rx.c
index XXXXXXX..XXXXXXX 100644
--- a/drivers/net/wireless/ath/carl9170/rx.c
+++ b/drivers/net/wireless/ath/carl9170/rx.c
@@ -151,7 +151,8 @@ static void carl9170_cmd_callback(struct ar9170 *ar, u32 len, void *buffer)
 	spin_lock(&ar->cmd_lock);
 	if (ar->readbuf) {
 		if (len >= 4)
-			memcpy(ar->readbuf, buffer + 4, len - 4);
+			memcpy(ar->readbuf, buffer + 4,
+			       min_t(u32, len - 4, ar->readlen));

 		ar->readbuf = NULL;
 	}
--
2.43.0

[PATCH v2 2/3] wifi: carl9170: fix OOB read from off-by-two in TX status handler

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

The bounds check in carl9170_tx_process_status() uses
`i > ((cmd->hdr.len / 2) + 1)` which is off by two, allowing
2 extra iterations past valid _tx_status entries when the firmware-
controlled hdr.ext exceeds hdr.len/2. Fix by using the correct
comparison `i >= (cmd->hdr.len / 2)`.

Fixes: a84fab3cbfdc ("carl9170: 802.11 rx/tx processing and usb backend")
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
drivers/net/wireless/ath/carl9170/tx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/carl9170/tx.c b/drivers/net/wireless/ath/carl9170/tx.c
index XXXXXXX..XXXXXXX 100644
--- a/drivers/net/wireless/ath/carl9170/tx.c
+++ b/drivers/net/wireless/ath/carl9170/tx.c
@@ -695,7 +695,7 @@ static void carl9170_tx_process_status(struct ar9170 *ar,
 	unsigned int i;

 	for (i = 0;  i < cmd->hdr.ext; i++) {
-		if (WARN_ON(i > ((cmd->hdr.len / 2) + 1))) {
+		if (WARN_ON(i >= (cmd->hdr.len / 2))) {
 			print_hex_dump_bytes("UU:", DUMP_PREFIX_NONE,
 					     (void *) cmd, cmd->hdr.len + 4);
 			break;

[PATCH v2 3/3] wifi: carl9170: fix buffer overflow in rx_stream failover path

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

The failover continuation in carl9170_rx_stream() copies the full tlen
from the second USB transfer instead of capping at rx_failover_missing
bytes. When both transfers are near maximum size, the total exceeds the
65535-byte failover SKB, triggering skb_over_panic.

Limit the copy size to the missing byte count.

Fixes: a84fab3cbfdc ("carl9170: 802.11 rx/tx processing and usb backend")
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
drivers/net/wireless/ath/carl9170/rx.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/carl9170/rx.c b/drivers/net/wireless/ath/carl9170/rx.c
index XXXXXXX..XXXXXXX 100644
--- a/drivers/net/wireless/ath/carl9170/rx.c
+++ b/drivers/net/wireless/ath/carl9170/rx.c
@@ -918,7 +918,9 @@ static void carl9170_rx_stream(struct ar9170 *ar, void *buf, unsigned int len)
 			}
 		}

-		skb_put_data(ar->rx_failover, tbuf, tlen);
+		skb_put_data(ar->rx_failover, tbuf,
+			     min_t(unsigned int, tlen,
+				   ar->rx_failover_missing));
 		ar->rx_failover_missing -= tlen;

 		if (ar->rx_failover_missing <= 0) {