Linux wireless drivers development
 help / color / mirror / Atom feed
* [PATCH] wifi: rtw88: fix skb leak in hw_scan probe request length check
@ 2026-11-13 15:17 Mauricio Faria de Oliveira
  2026-11-14 13:40 ` Luka Gejak
  0 siblings, 1 reply; 4+ messages in thread
From: Mauricio Faria de Oliveira @ 2026-11-13 15:17 UTC (permalink / raw)
  To: linux-wireless
  Cc: Ping-Ke Shih, Po-Hao Huang, Kalle Valo, kernel-dev, linux-kernel,
	Mauricio Faria de Oliveira

In _rtw_hw_scan_update_probe_req(), if the first loop fails (goto out;) the
skb is leaked. It does not reach kfree_skb() at the end of the loop body or
in the second loop (out:) since it is unlinked from probe_req_list.

Fix the leak by calling kfree_skb() if the first loop fails.

In /sys/kernel/debug/kmemleak, after simulating the failure:

  unreferenced object 0xffff88810965a000 (size 232):
    comm "iw", pid 4729, jiffies 4345008185
    hex dump (first 32 bytes):
      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
    backtrace (crc 20f7de11):
      kmem_cache_alloc_node_noprof+0x302/0x380
      __alloc_skb+0x74/0x2f0
      skb_copy+0x57/0xe0
      rtw_hw_scan_update_probe_req+0xff/0x230
      rtw_hw_scan_prehandle+0x4e/0x180
      rtw_hw_scan_offload+0x186/0x1d0
      rtw_ops_hw_scan+0x61/0xb0
      drv_hw_scan+0x81/0x130
      __ieee80211_start_scan+0x2e0/0x760
      rdev_scan+0x42/0x100
      nl80211_trigger_scan+0x53b/0x730
      genl_family_rcv_msg_doit+0x103/0x160
      genl_rcv_msg+0x1b5/0x2c0
      netlink_rcv_skb+0x52/0x100
      genl_rcv+0x28/0x40
      netlink_unicast+0x26d/0x3b0

This leak exists in the first loop since the function was introduced, and
still happened with the addition of the second loop, in these two commits:

Fixes: 10d162b2ed39 ("rtw88: 8822c: add ieee80211_ops::hw_scan")
Fixes: d95984b5580d ("rtw88: fix memory overrun and memory leak during hw_scan")
Signed-off-by: Mauricio Faria de Oliveira <mfo@igalia.com>
---
 drivers/net/wireless/realtek/rtw88/fw.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/realtek/rtw88/fw.c b/drivers/net/wireless/realtek/rtw88/fw.c
index 945fedcd375b62b4110ab631e87889bb84e2f16b..09b5836540c41e80a5fecca0f781aa45c6327cc0 100644
--- a/drivers/net/wireless/realtek/rtw88/fw.c
+++ b/drivers/net/wireless/realtek/rtw88/fw.c
@@ -2061,6 +2061,7 @@ static int _rtw_hw_scan_update_probe_req(struct rtw_dev *rtwdev, u8 num_probes,
 		skb_unlink(skb, probe_req_list);
 		rtw_fill_rsvd_page_desc(rtwdev, skb, RSVD_PROBE_REQ);
 		if (skb->len > page_size * page_cnt) {
+			kfree_skb(skb);
 			ret = -EINVAL;
 			goto out;
 		}

---
base-commit: 7e1392a42ea288d70a266f6553fe06f57f8647ea
change-id: 20261112-rtw88-hw-scan-skb-leak-9e7154def3d5

Best regards,
-- 
Mauricio Faria de Oliveira <mfo@igalia.com>


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] wifi: rtw88: fix skb leak in hw_scan probe request length check
  2026-11-13 15:17 [PATCH] wifi: rtw88: fix skb leak in hw_scan probe request length check Mauricio Faria de Oliveira
@ 2026-11-14 13:40 ` Luka Gejak
  2026-11-14 17:06   ` Thadeu Lima de Souza Cascardo
  0 siblings, 1 reply; 4+ messages in thread
From: Luka Gejak @ 2026-11-14 13:40 UTC (permalink / raw)
  To: linux-wireless, Mauricio Faria de Oliveira
  Cc: Ping-Ke Shih, Po-Hao Huang, Kalle Valo, kernel-dev, linux-kernel

On November 13, 2026 4:17:33 PM GMT+01:00, Mauricio Faria de Oliveira
<mfo@igalia.com> wrote:
>In _rtw_hw_scan_update_probe_req(), if the first loop fails (goto out;)
>the
>skb is leaked. It does not reach kfree_skb() at the end of the loop body
>or
>in the second loop (out:) since it is unlinked from probe_req_list.
>
>Fix the leak by calling kfree_skb() if the first loop fails.
>
>In /sys/kernel/debug/kmemleak, after simulating the failure:
>
>  unreferenced object 0xffff88810965a000 (size 232):
>    comm "iw", pid 4729, jiffies 4345008185
>    hex dump (first 32 bytes):
>      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>    backtrace (crc 20f7de11):
>      kmem_cache_alloc_node_noprof+0x302/0x380
>      __alloc_skb+0x74/0x2f0
>      skb_copy+0x57/0xe0
>      rtw_hw_scan_update_probe_req+0xff/0x230
>      rtw_hw_scan_prehandle+0x4e/0x180
>      rtw_hw_scan_offload+0x186/0x1d0
>      rtw_ops_hw_scan+0x61/0xb0
>      drv_hw_scan+0x81/0x130
>      __ieee80211_start_scan+0x2e0/0x760
>      rdev_scan+0x42/0x100
>      nl80211_trigger_scan+0x53b/0x730
>      genl_family_rcv_msg_doit+0x103/0x160
>      genl_rcv_msg+0x1b5/0x2c0
>      netlink_rcv_skb+0x52/0x100
>      genl_rcv+0x28/0x40
>      netlink_unicast+0x26d/0x3b0
>

Should this dump go under --- so it doesn't end up in commit message?

>This leak exists in the first loop since the function was introduced, and
>still happened with the addition of the second loop, in these two commits:
>
>Fixes: 10d162b2ed39 ("rtw88: 8822c: add ieee80211_ops::hw_scan")
>Fixes: d95984b5580d ("rtw88: fix memory overrun and memory leak during hw_scan")

Should this perhaps cc stable?

>Signed-off-by: Mauricio Faria de Oliveira <mfo@igalia.com>
>---

Either way I'm fine with the code, so just fix the commit message. Feel
free to carry the tag for v2.

Reviewed-by: Luka Gejak <luka.gejak@linux.dev>

Best regards,
Luka Gejak

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] wifi: rtw88: fix skb leak in hw_scan probe request length check
  2026-11-14 13:40 ` Luka Gejak
@ 2026-11-14 17:06   ` Thadeu Lima de Souza Cascardo
  2026-11-14 17:17     ` Luka Gejak
  0 siblings, 1 reply; 4+ messages in thread
From: Thadeu Lima de Souza Cascardo @ 2026-11-14 17:06 UTC (permalink / raw)
  To: Luka Gejak
  Cc: linux-wireless, Mauricio Faria de Oliveira, Ping-Ke Shih,
	Po-Hao Huang, Kalle Valo, kernel-dev, linux-kernel

On Sat, Nov 14, 2026 at 02:40:24PM +0100, Luka Gejak wrote:
> On November 13, 2026 4:17:33 PM GMT+01:00, Mauricio Faria de Oliveira
> <mfo@igalia.com> wrote:
> >In _rtw_hw_scan_update_probe_req(), if the first loop fails (goto out;)
> >the
> >skb is leaked. It does not reach kfree_skb() at the end of the loop body
> >or
> >in the second loop (out:) since it is unlinked from probe_req_list.
> >
> >Fix the leak by calling kfree_skb() if the first loop fails.
> >
> >In /sys/kernel/debug/kmemleak, after simulating the failure:
> >
> >  unreferenced object 0xffff88810965a000 (size 232):
> >    comm "iw", pid 4729, jiffies 4345008185
> >    hex dump (first 32 bytes):
> >      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> >      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> >    backtrace (crc 20f7de11):
> >      kmem_cache_alloc_node_noprof+0x302/0x380
> >      __alloc_skb+0x74/0x2f0
> >      skb_copy+0x57/0xe0
> >      rtw_hw_scan_update_probe_req+0xff/0x230
> >      rtw_hw_scan_prehandle+0x4e/0x180
> >      rtw_hw_scan_offload+0x186/0x1d0
> >      rtw_ops_hw_scan+0x61/0xb0
> >      drv_hw_scan+0x81/0x130
> >      __ieee80211_start_scan+0x2e0/0x760
> >      rdev_scan+0x42/0x100
> >      nl80211_trigger_scan+0x53b/0x730
> >      genl_family_rcv_msg_doit+0x103/0x160
> >      genl_rcv_msg+0x1b5/0x2c0
> >      netlink_rcv_skb+0x52/0x100
> >      genl_rcv+0x28/0x40
> >      netlink_unicast+0x26d/0x3b0
> >
> 
> Should this dump go under --- so it doesn't end up in commit message?
> 

I find stack traces particularly helpful in commit messages. The help find
out fixes for bugs one encounter, avoiding duplicate work, and they also
justify the commit.

Cascardo.

> >This leak exists in the first loop since the function was introduced, and
> >still happened with the addition of the second loop, in these two commits:
> >
> >Fixes: 10d162b2ed39 ("rtw88: 8822c: add ieee80211_ops::hw_scan")
> >Fixes: d95984b5580d ("rtw88: fix memory overrun and memory leak during hw_scan")
> 
> Should this perhaps cc stable?
> 
> >Signed-off-by: Mauricio Faria de Oliveira <mfo@igalia.com>
> >---
> 
> Either way I'm fine with the code, so just fix the commit message. Feel
> free to carry the tag for v2.
> 
> Reviewed-by: Luka Gejak <luka.gejak@linux.dev>
> 
> Best regards,
> Luka Gejak

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] wifi: rtw88: fix skb leak in hw_scan probe request length check
  2026-11-14 17:06   ` Thadeu Lima de Souza Cascardo
@ 2026-11-14 17:17     ` Luka Gejak
  0 siblings, 0 replies; 4+ messages in thread
From: Luka Gejak @ 2026-11-14 17:17 UTC (permalink / raw)
  To: Thadeu Lima de Souza Cascardo
  Cc: linux-wireless, Mauricio Faria de Oliveira, Ping-Ke Shih,
	Po-Hao Huang, Kalle Valo, kernel-dev, linux-kernel

On November 14, 2026 6:06:18 PM GMT+01:00, Thadeu Lima de Souza Cascardo
<cascardo@igalia.com> wrote:
>On Sat, Nov 14, 2026 at 02:40:24PM +0100, Luka Gejak wrote:
>> On November 13, 2026 4:17:33 PM GMT+01:00, Mauricio Faria de Oliveira
>> <mfo@igalia.com> wrote:
>> >In _rtw_hw_scan_update_probe_req(), if the first loop fails (goto out;)
>> >the
>> >skb is leaked. It does not reach kfree_skb() at the end of the loop
>body
>> >or
>> >in the second loop (out:) since it is unlinked from probe_req_list.
>> >
>> >Fix the leak by calling kfree_skb() if the first loop fails.
>> >
>> >In /sys/kernel/debug/kmemleak, after simulating the failure:
>> >
>> >  unreferenced object 0xffff88810965a000 (size 232):
>> >    comm "iw", pid 4729, jiffies 4345008185
>> >    hex dump (first 32 bytes):
>> >      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>> >      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>> >    backtrace (crc 20f7de11):
>> >      kmem_cache_alloc_node_noprof+0x302/0x380
>> >      __alloc_skb+0x74/0x2f0
>> >      skb_copy+0x57/0xe0
>> >      rtw_hw_scan_update_probe_req+0xff/0x230
>> >      rtw_hw_scan_prehandle+0x4e/0x180
>> >      rtw_hw_scan_offload+0x186/0x1d0
>> >      rtw_ops_hw_scan+0x61/0xb0
>> >      drv_hw_scan+0x81/0x130
>> >      __ieee80211_start_scan+0x2e0/0x760
>> >      rdev_scan+0x42/0x100
>> >      nl80211_trigger_scan+0x53b/0x730
>> >      genl_family_rcv_msg_doit+0x103/0x160
>> >      genl_rcv_msg+0x1b5/0x2c0
>> >      netlink_rcv_skb+0x52/0x100
>> >      genl_rcv+0x28/0x40
>> >      netlink_unicast+0x26d/0x3b0
>> >
>> 
>> Should this dump go under --- so it doesn't end up in commit message?
>> 
>
>I find stack traces particularly helpful in commit messages. The help find
>out fixes for bugs one encounter, avoiding duplicate work, and they also
>justify the commit.
>
>Cascardo.
>

Personally I don't find it that necessary, as if I need it I will just
search the commit subject on the lore and get it from there, but I guess it
can't hurt to keep it in, so keep it if have a strong preference on that
matter.

Best regards,
Luka Gejak

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-11-14 17:17 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-13 15:17 [PATCH] wifi: rtw88: fix skb leak in hw_scan probe request length check Mauricio Faria de Oliveira
2026-11-14 13:40 ` Luka Gejak
2026-11-14 17:06   ` Thadeu Lima de Souza Cascardo
2026-11-14 17:17     ` Luka Gejak

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox