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

[Loic Poulain]

Hi Jeff,

On Fri, Jun 5, 2026 at 3:43 AM Jeff Johnson
<jeff.johnson at oss.qualcomm.com> wrote:
>
> On 4/21/2026 6:50 AM, Tristan Madani wrote:
> > From: Tristan Madani <tristan at 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 at talencesecurity.com>
> > ---
> > Changes in v3:
> >   - Regenerated from wireless-next with proper git format-patch to
> >     produce valid index hashes (v2 had post-processed index lines).
> >
> > Changes in v2:
> >   - No code changes from v1.
> >
> >  drivers/net/wireless/ath/wcn36xx/smd.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
> > index 813553edcb789..f65328329f4f0 100644
> > --- a/drivers/net/wireless/ath/wcn36xx/smd.c
> > +++ b/drivers/net/wireless/ath/wcn36xx/smd.c
> > @@ -3293,6 +3293,10 @@ int wcn36xx_smd_rsp_process(struct rpmsg_device *rpdev,
> >       case WCN36XX_HAL_EXIT_IMPS_RSP:
> >       case WCN36XX_HAL_UPDATE_CHANNEL_LIST_RSP:
> >       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);
>
> AI review points out that this logic will bypass the complete() meaning
> callers waiting for completion will be stuck (either forever or until the
> specified timeout expires). It proposes setting len = 0 instead of break and
> having each waiter deal with the issue that wcn->hal_rsp_len is 0.
>
> Further probing gave the observation that there is only one waiter
> wcn36xx_smd_send_and_wait() so there isn't a "wait forever" scenario.
>
> It also confirmed that setting wcn->hal_rsp_len = 0 would subsequently be
> processed by wcn36xx_smd_rsp_status_check() which would return -EIO due to len
> < sizeof(header) + sizeof(status_rsp).
>
> So setting len = 0 and still calling complete() would avoid the timeout and
> would cause -EIO vs -ETIME to be propagated.
>
> I can go either way with this since it is not expected to occur.

Thanks, the above analysis is indeed correct, but let's make it simple
and go as is, with the timeout.

Regards,
Loic