Re: [PATCH ath-next] wifi: wcn36xx: fix OOB read from short firmware HAL message

[Jeff Johnson]

On 7/25/2026 11:57 AM, Shitalkumar Gandhi wrote:
> wcn36xx_smd_rsp_process() is the rpmsg callback for the WLAN_CTRL
> channel. It dereferences msg_header->msg_type to dispatch the incoming
> message before validating that the payload is actually large enough to
> contain a struct wcn36xx_hal_msg_header.
> 
> The payload length is fully controlled by the remote processor: the SMD
> transport takes it from the packet header written by WCNSS
> (qcom_smd_channel_intr() stores le32_to_cpu(pktlen) into pkt_size) and
> hands it to the callback unchecked. A message shorter than the 8-byte
> header therefore causes an out-of-bounds read. For the indication
> message types the header is read a second time in wcn36xx_ind_smd_work(),
> there from a kmalloc_flex() allocation sized to the short length, which
> is a slab out-of-bounds read that also violates the
> __counted_by(msg_len) annotation on struct wcn36xx_hal_ind_msg.
> 
> The individual response and indication handlers do validate their own
> lengths, but they only run after this dispatch has already happened.

this is not actually true, see my comment at the very end

> 
> Add the missing header length check. Drop the message and return 0 so
> the SMD transport still advances its receive FIFO tail; returning an
> error would leave the malformed packet unconsumed and stall the channel,
> which is why the oversized-response case in the same function also
> consumes the message.

this is a very lengthy commit description that looks LLM generated, and yet
there is no Assisted-by tag. See current guidance at:
https://docs.kernel.org/process/coding-assistants.html

Note that a simple 4-line patch that matches the logic in a lot of drivers
doesn't need this level of detail

> 
> Fixes: 8e84c2582169 ("wcn36xx: mac80211 driver for Qualcomm WCN3660/WCN3680 hardware")
> Signed-off-by: Shitalkumar Gandhi <shitalkumar.gandhi@xxxxxxxxxxxxxxxxxxx>

checkpatch warns:
WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address mismatch:
'From: Shitalkumar Gandhi <shital.gandhi45@xxxxxxxxx>' != 'Signed-off-by:
Shitalkumar Gandhi <shitalkumar.gandhi@xxxxxxxxxxxxxxxxxxx>'

It looks like gmail may have mangled your From address based upon other tags
in the e-mail (X-Google-Original-From:) so you need to work around this.

I can't take a patch where SOB != From (although I see in lore that some
maintainers have incorrectly taken some of your patches!)

> ---
>  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 c0b477345832..32dec45dbb50 100644
> --- a/drivers/net/wireless/ath/wcn36xx/smd.c
> +++ b/drivers/net/wireless/ath/wcn36xx/smd.c
> @@ -3254,6 +3254,11 @@ int wcn36xx_smd_rsp_process(struct rpmsg_device *rpdev,
>  	struct wcn36xx_hal_ind_msg *msg_ind;
>  	wcn36xx_dbg_dump(WCN36XX_DBG_SMD_DUMP, "SMD <<< ", buf, len);
>  
> +	if (len < sizeof(*msg_header)) {
> +		wcn36xx_warn("Corrupted HAL message: length %d\n", len);
> +		return 0;
> +	}
> +
>  	switch (msg_header->msg_type) {
>  	case WCN36XX_HAL_START_RSP:
>  	case WCN36XX_HAL_CONFIG_STA_RSP:
> 

> base-commit: 90883c513ca01e77c56b8d94be7de11420c1dacf
As noted earlier there are other buffer over-reads in the wcn36xx SMD rx
processing? My code review bot is flagging:
wcn36xx_smd_join_rsp()
wcn36xx_smd_switch_channel_rsp()
wcn36xx_smd_get_stats()
wcn36xx_smd_gtk_offload_get_info_rsp()
wcn36xx_smd_process_ptt_msg_rsp()

if you decide to address these please do each as a separate patch

/jeff