[PATCH] wifi: carl9170: reject mismatched command response lengths

[PATCH] wifi: carl9170: reject mismatched command response lengths

[Yousef Alhouseen]

The firmware response length is controlled by the USB device. Although
carl9170_cmd_callback() detects when it differs from the output buffer
length, the function falls through and copies the entire response into
that buffer. Callers commonly provide stack objects, so a malformed
response can overwrite the kernel stack.

Return after scheduling device recovery. This also preserves the stated
behavior of leaving the command incomplete so that its waiter times out
and clears the pending output buffer.

Fixes: a84fab3cbfdc ("carl9170: 802.11 rx/tx processing and usb backend")
Reported-by: syzbot+5c1ca6ccaa1215781cac@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=5c1ca6ccaa1215781cac
Cc: stable@vger.kernel.org
Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
---
 drivers/net/wireless/ath/carl9170/rx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/ath/carl9170/rx.c b/drivers/net/wireless/ath/carl9170/rx.c
index 6833430130f4..ea3f435fb64c 100644
--- a/drivers/net/wireless/ath/carl9170/rx.c
+++ b/drivers/net/wireless/ath/carl9170/rx.c
@@ -145,6 +145,7 @@ static void carl9170_cmd_callback(struct ar9170 *ar, u32 len, void *buffer)
 		 * and we get a stack trace from there.
 		 */
 		carl9170_restart(ar, CARL9170_RR_INVALID_RSP);
+		return;
 	}
 
 	spin_lock(&ar->cmd_lock);
-- 
2.54.0

Re: [PATCH] wifi: carl9170: reject mismatched command response lengths

[Christian Lamparter]

Hi,

On 6/28/26 11:28 AM, Yousef Alhouseen wrote:
> The firmware response length is controlled by the USB device. Although
> carl9170_cmd_callback() detects when it differs from the output buffer
> length, the function falls through and copies the entire response into
> that buffer. Callers commonly provide stack objects, so a malformed
> response can overwrite the kernel stack.
> 
> Return after scheduling device recovery. This also preserves the stated
> behavior of leaving the command incomplete so that its waiter times out
> and clears the pending output buffer.
> 
> Fixes: a84fab3cbfdc ("carl9170: 802.11 rx/tx processing and usb backend")
> Reported-by: syzbot+5c1ca6ccaa1215781cac@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=5c1ca6ccaa1215781cac
> Cc: stable@vger.kernel.org
> Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>


This was posted earlier too:
https://lore.kernel.org/linux-wireless/20260421134929.325662-1-tristmd@gmail.com/

In fact, there was even a mail before that that was sent to security@vger.kernel.org.
I told Tristan that I would much rather not return and instead fix the memcpy.
carl9170_restart can completely unbind the device, so it's unlikely that one would
see a timeout.

Cheers,
Christian


> ---
>   drivers/net/wireless/ath/carl9170/rx.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/wireless/ath/carl9170/rx.c b/drivers/net/wireless/ath/carl9170/rx.c
> index 6833430130f4..ea3f435fb64c 100644
> --- a/drivers/net/wireless/ath/carl9170/rx.c
> +++ b/drivers/net/wireless/ath/carl9170/rx.c
> @@ -145,6 +145,7 @@ static void carl9170_cmd_callback(struct ar9170 *ar, u32 len, void *buffer)
>   		 * and we get a stack trace from there.
>   		 */
>   		carl9170_restart(ar, CARL9170_RR_INVALID_RSP);
> +		return;
>   	}
>   
>   	spin_lock(&ar->cmd_lock);

Re: [PATCH] wifi: carl9170: reject mismatched command response lengths

[Yousef Alhouseen]

Hi Christian,

Thanks for pointing out Tristan's earlier patch and the prior security
discussion. I had not found that submission before sending mine.

I agree that returning after carl9170_restart() is not the right fix
if the restart can unbind the device, and that the copy itself should
be bounded instead. Please drop my patch; I will defer to the existing
discussion and will not send a v2.

Thanks,
Yousef

On Sat, 4 Jul 2026 21:56:30 +0200, Christian Lamparter
<chunkeey@gmail.com> wrote:
> Hi,
>
> On 6/28/26 11:28 AM, Yousef Alhouseen wrote:
> > The firmware response length is controlled by the USB device. Although
> > carl9170_cmd_callback() detects when it differs from the output buffer
> > length, the function falls through and copies the entire response into
> > that buffer. Callers commonly provide stack objects, so a malformed
> > response can overwrite the kernel stack.
> >
> > Return after scheduling device recovery. This also preserves the stated
> > behavior of leaving the command incomplete so that its waiter times out
> > and clears the pending output buffer.
> >
> > Fixes: a84fab3cbfdc ("carl9170: 802.11 rx/tx processing and usb backend")
> > Reported-by: syzbot+5c1ca6ccaa1215781cac@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=5c1ca6ccaa1215781cac
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
>
> This was posted earlier too:
> https://lore.kernel.org/linux-wireless/20260421134929.325662-1-tristmd@gmail.com/
>
> In fact, there was even a mail before that that was sent to security@vger.kernel.org.
> I told Tristan that I would much rather not return and instead fix the memcpy.
> carl9170_restart can completely unbind the device, so it's unlikely that one would
> see a timeout.
>
> Cheers,
> Christian
>
> > ---
> > drivers/net/wireless/ath/carl9170/rx.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/net/wireless/ath/carl9170/rx.c b/drivers/net/wireless/ath/carl9170/rx.c
> > index 6833430130f4..ea3f435fb64c 100644
> > --- a/drivers/net/wireless/ath/carl9170/rx.c
> > +++ b/drivers/net/wireless/ath/carl9170/rx.c
> > @@ -145,6 +145,7 @@ static void carl9170_cmd_callback(struct ar9170 *ar, u32 len, void *buffer)
> > * and we get a stack trace from there.
> > */
> > carl9170_restart(ar, CARL9170_RR_INVALID_RSP);
> > + return;
> > }
> >
> > spin_lock(&ar->cmd_lock);