[PATCH v2 0/3] wifi: wcn36xx: fix OOB reads and heap overflow from firmware responses

[PATCH v2 0/3] wifi: wcn36xx: fix OOB reads and heap overflow from firmware responses

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

Hi Loic,

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 wcn36xx HAL firmware response handling, including a heap
overflow in the main response dispatcher:

Proposed fixes in the following patches.

Thanks,
Tristan

[PATCH v2 0/3] wifi: wcn36xx: fix OOB reads and heap overflow from firmware responses

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

Hi Loic,

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 wcn36xx HAL firmware response handling, including a heap
overflow in the main response dispatcher:

Proposed fixes in the following patches.

Thanks,
Tristan

[PATCH v2 1/3] wifi: wcn36xx: fix heap overflow from oversized firmware HAL response

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

The firmware response dispatcher copies all synchronous HAL responses
into the 4096-byte hal_buf without validating the response length. A
response exceeding WCN36XX_HAL_BUF_SIZE causes a heap buffer overflow
with firmware-controlled content.

Add a bounds check on the response length.

Fixes: 8e84c2582169 ("wcn36xx: mac80211 driver for Qualcomm WCN3660/WCN3680 hardware")
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
drivers/net/wireless/ath/wcn36xx/smd.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index XXXXXXX..XXXXXXX 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -3296,6 +3296,11 @@ int wcn36xx_smd_rsp_process(struct rpmsg_device *rpdev,
 	case WCN36XX_HAL_ADD_BCN_FILTER_RSP:
+		if (len > WCN36XX_HAL_BUF_SIZE) {
+			wcn36xx_warn("HAL response too large: %d\n", len);
+			break;
+		}
 		memcpy(wcn->hal_buf, buf, len);
 		wcn->hal_rsp_len = len;
 		complete(&wcn->hal_rsp_compl);

[PATCH v2 2/3] wifi: wcn36xx: fix OOB read from firmware count in PRINT_REG_INFO indication

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

The firmware-controlled rsp->count field is used as the loop bound for
indexing into the flexible rsp->regs[] array without validation against
the message length. A count exceeding the actual data causes out-of-
bounds reads from the heap-allocated message buffer.

Add a check that count fits within the received message.

Fixes: 43efa3c0f241 ("wcn36xx: Implement print_reg indication")
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
drivers/net/wireless/ath/wcn36xx/smd.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index XXXXXXX..XXXXXXX 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -2803,6 +2803,12 @@ static int wcn36xx_smd_print_reg_info_ind(struct wcn36xx *wcn,
 		return -EIO;
 	}

+	if (rsp->count > (len - sizeof(*rsp)) / sizeof(rsp->regs[0])) {
+		wcn36xx_warn("Truncated print reg info indication: count %u, len %zu\n",
+			     rsp->count, len);
+		return -EIO;
+	}
+
 	wcn36xx_dbg(WCN36XX_DBG_HAL,
 		    "reginfo indication, scenario: 0x%x reason: 0x%x\n",
 		    rsp->scenario, rsp->reason);

[PATCH v2 3/3] wifi: wcn36xx: fix OOB read from short trigger BA firmware response

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

The firmware response length is only checked against sizeof(*rsp) (20
bytes), but when candidate_cnt >= 1, a 22-byte candidate struct is read
at buf + 20 without verifying the response contains it. This causes an
out-of-bounds read of stale heap data, corrupting the BA session state.

Add validation that the response includes the candidate data.

Fixes: 16be1ac55944 ("wcn36xx: Parse trigger_ba response properly")
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
drivers/net/wireless/ath/wcn36xx/smd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index XXXXXXX..XXXXXXX 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -2599,6 +2599,9 @@ static int wcn36xx_smd_trigger_ba_rsp(void *buf, int len, struct add_ba_info *ba
 	if (rsp->candidate_cnt < 1)
 		return rsp->status ? rsp->status : -EINVAL;

+	if (len < sizeof(*rsp) + sizeof(*candidate))
+		return -EINVAL;
+
 	candidate = (struct wcn36xx_hal_trigger_ba_rsp_candidate *)(buf + sizeof(*rsp));

 	for (i = 0; i < STACFG_MAX_TC; i++) {

Re: [PATCH v2 1/3] wifi: wcn36xx: fix heap overflow from oversized firmware HAL response

[Johannes Berg]

Hi Tristan,

On Wed, 2026-04-15 at 22:37 +0000, Tristan Madani wrote:
> From: Tristan Madani <tristan@talencesecurity.com>
> 
> The firmware response dispatcher copies all synchronous HAL responses
> into the 4096-byte hal_buf without validating the response length. A
> response exceeding WCN36XX_HAL_BUF_SIZE causes a heap buffer overflow
> with firmware-controlled content.
> 
> Add a bounds check on the response length.

No real problem with these patches etc., but it seems implausible that
you're not using some kind of tool/LLM assistance, which you're supposed
to disclose (or at least I guess I'm supposed to ask you to):

https://docs.kernel.org/process/coding-assistants.html

johannes

Re: [PATCH v2 0/3] wifi: wcn36xx: fix OOB reads and heap overflow from firmware responses

[Jeff Johnson]

On 4/15/2026 3:37 PM, Tristan Madani wrote:
> From: Tristan Madani <tristan@talencesecurity.com>
> 
> Hi Loic,
> 
> 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 wcn36xx HAL firmware response handling, including a heap
> overflow in the main response dispatcher:
> 
> Proposed fixes in the following patches.
> 
> Thanks,
> Tristan

Are you able to cause these issues to occur?
My expectation is that this is testing things that firmware will never do, and
hence this adds code and processing with no actual benefit.

/jeff

Re: [PATCH v2 0/3] wifi: wcn36xx: fix OOB reads and heap overflow from firmware responses

[Johannes Berg]

On Thu, 2026-04-16 at 09:25 -0700, Jeff Johnson wrote:
> On 4/15/2026 3:37 PM, Tristan Madani wrote:
> > From: Tristan Madani <tristan@talencesecurity.com>
> > 
> > Hi Loic,
> > 
> > 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 wcn36xx HAL firmware response handling, including a heap
> > overflow in the main response dispatcher:
> > 
> > Proposed fixes in the following patches.
> > 
> > Thanks,
> > Tristan
> 
> Are you able to cause these issues to occur?
> My expectation is that this is testing things that firmware will never do, and
> hence this adds code and processing with no actual benefit.

We're not really supposed to completely trust firmware though, right? :)

johannes

Re: [PATCH v2 0/3] wifi: wcn36xx: fix OOB reads and heap overflow from firmware responses

[Loic Poulain]

On Thu, Apr 16, 2026 at 8:39 PM Johannes Berg <johannes@sipsolutions.net> wrote:
>
> On Thu, 2026-04-16 at 09:25 -0700, Jeff Johnson wrote:
> > On 4/15/2026 3:37 PM, Tristan Madani wrote:
> > > From: Tristan Madani <tristan@talencesecurity.com>
> > >
> > > Hi Loic,
> > >
> > > 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 wcn36xx HAL firmware response handling, including a heap
> > > overflow in the main response dispatcher:
> > >
> > > Proposed fixes in the following patches.
> > >
> > > Thanks,
> > > Tristan
> >
> > Are you able to cause these issues to occur?
> > My expectation is that this is testing things that firmware will never do, and
> > hence this adds code and processing with no actual benefit.
>
> We're not really supposed to completely trust firmware though, right? :)

Right, at first glance, these appear to be legitimate and
straightforward buffer boundary checks. I’ll follow up with a review.

Regards,
Loic

Re: [PATCH v2 0/3] wifi: wcn36xx: fix OOB reads and heap overflow from firmware responses

[Jeff Johnson]

On 4/16/2026 11:39 AM, Johannes Berg wrote:
> On Thu, 2026-04-16 at 09:25 -0700, Jeff Johnson wrote:
>> On 4/15/2026 3:37 PM, Tristan Madani wrote:
>>> From: Tristan Madani <tristan@talencesecurity.com>
>>>
>>> Hi Loic,
>>>
>>> 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 wcn36xx HAL firmware response handling, including a heap
>>> overflow in the main response dispatcher:
>>>
>>> Proposed fixes in the following patches.
>>>
>>> Thanks,
>>> Tristan
>>
>> Are you able to cause these issues to occur?
>> My expectation is that this is testing things that firmware will never do, and
>> hence this adds code and processing with no actual benefit.
> 
> We're not really supposed to completely trust firmware though, right? :)

Like everything else in software there are tradeoffs. You have to mostly trust
firmware since everything it it is doing is on behalf of the driver. So that
is why I'm curious if these issues are actually exploitable, or if this is
just preventative for the sake of being preventative.

/jeff

Re: [PATCH v2 0/3] wifi: wcn36xx: fix OOB reads and heap overflow from firmware responses

[Loic Poulain]

Hi Jeff,

On Fri, Apr 17, 2026 at 2:01 AM Jeff Johnson
<jeff.johnson@oss.qualcomm.com> wrote:
>
> On 4/16/2026 11:39 AM, Johannes Berg wrote:
> > On Thu, 2026-04-16 at 09:25 -0700, Jeff Johnson wrote:
> >> On 4/15/2026 3:37 PM, Tristan Madani wrote:
> >>> From: Tristan Madani <tristan@talencesecurity.com>
> >>>
> >>> Hi Loic,
> >>>
> >>> 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 wcn36xx HAL firmware response handling, including a heap
> >>> overflow in the main response dispatcher:
> >>>
> >>> Proposed fixes in the following patches.
> >>>
> >>> Thanks,
> >>> Tristan
> >>
> >> Are you able to cause these issues to occur?
> >> My expectation is that this is testing things that firmware will never do, and
> >> hence this adds code and processing with no actual benefit.
> >
> > We're not really supposed to completely trust firmware though, right? :)
>
> Like everything else in software there are tradeoffs. You have to mostly trust
> firmware since everything it it is doing is on behalf of the driver. So that
> is why I'm curious if these issues are actually exploitable, or if this is
> just preventative for the sake of being preventative.

I agree that some degree of trust in vendor firmware is unavoidable,
as its internal behavior cannot be fully controlled or inspected.
Nevertheless, the kernel and its drivers are expected to strictly
validate and constrain all interactions with firmware, so that any
faulty or compromised behavior is contained within its intended scope
(network/wireless).

So, it is the driver’s responsibility to control the firmware
interface and ensure that firmware misbehavior, whether deliberate or
the result of an exploited firmware bug, cannot lead to such kernel
memory corruption, which could otherwise be exploited well beyond the
driver’s original functional domain.

While this issue may be largely theoretical, it has now been reported
and a fix is available. In this context, addressing it seems to be the
responsible course of action.

With the increasing use of AI, we may indeed see more fixes proposed
for issues that are theoretical or unlikely to occur in practice. I
understand the concern about avoiding an influx of low‑value commits
and the need for some level of filtering. That said, in this particular case,
I believe the fix is legitimate.

Regards,
Loic