All of lore.kernel.org
 help / color / mirror / Atom feed
* brcmfmac: fix use-after-free bug
@ 2022-08-02 17:28 Alexander Coffin
  2022-08-02 17:28 ` [PATCH] " Alexander Coffin
  0 siblings, 1 reply; 9+ messages in thread
From: Alexander Coffin @ 2022-08-02 17:28 UTC (permalink / raw)
  To: Arend van Spriel, Franky Lin, Hante Meuleman
  Cc: linux-wireless, brcm80211-dev-list.pdl, SHA-cyfmac-dev-list

The following use-after-free is addressed by this patch. I have not used git
send-email in a long time, and I have never contributed to the Linux kernel
before so I have you can forgive, and correct and mistakes that I may have made
in my process of submitting this patch.

[   46.912801] ==================================================================
[   46.920552] BUG: KASAN: use-after-free in brcmf_netdev_start_xmit+0x718/0x8c8 [brcmfmac]
[   46.928673] Read of size 4 at addr ffffff803f5882e8 by task systemd-resolve/328
[   46.935991] 
[   46.937514] CPU: 1 PID: 328 Comm: systemd-resolve Tainted: G           O      5.4.199-[REDACTED] #1
[   46.947255] Hardware name: [REDACTED]
[   46.954568] Call trace:
[   46.957037]  dump_backtrace+0x0/0x2b8
[   46.960719]  show_stack+0x24/0x30
[   46.964052]  dump_stack+0x128/0x194
[   46.967557]  print_address_description.isra.0+0x64/0x380
[   46.972877]  __kasan_report+0x1d4/0x240
[   46.976723]  kasan_report+0xc/0x18
[   46.980138]  __asan_report_load4_noabort+0x18/0x20
[   46.985027]  brcmf_netdev_start_xmit+0x718/0x8c8 [brcmfmac]
[   46.990613]  dev_hard_start_xmit+0x1bc/0xda0
[   46.994894]  sch_direct_xmit+0x198/0xd08
[   46.998827]  __qdisc_run+0x37c/0x1dc0
[   47.002500]  __dev_queue_xmit+0x1528/0x21f8
[   47.006692]  dev_queue_xmit+0x24/0x30
[   47.010366]  neigh_resolve_output+0x37c/0x678
[   47.014734]  ip_finish_output2+0x598/0x2458
[   47.018927]  __ip_finish_output+0x300/0x730
[   47.023118]  ip_output+0x2e0/0x430
[   47.026530]  ip_local_out+0x90/0x140
[   47.030117]  igmpv3_sendpack+0x14c/0x228
[   47.034049]  igmpv3_send_cr+0x384/0x6b8
[   47.037895]  igmp_ifc_timer_expire+0x4c/0x118
[   47.042262]  call_timer_fn+0x1cc/0xbe8
[   47.046021]  __run_timers+0x4d8/0xb28
[   47.049693]  run_timer_softirq+0x24/0x40
[   47.053626]  __do_softirq+0x2c0/0x117c
[   47.057387]  irq_exit+0x2dc/0x388
[   47.060715]  __handle_domain_irq+0xb4/0x158
[   47.064908]  gic_handle_irq+0x58/0xb0
[   47.068581]  el0_irq_naked+0x50/0x5c
[   47.072162] 
[   47.073665] Allocated by task 328:
[   47.077083]  save_stack+0x24/0xb0
[   47.080410]  __kasan_kmalloc.isra.0+0xc0/0xe0
[   47.084776]  kasan_slab_alloc+0x14/0x20
[   47.088622]  kmem_cache_alloc+0x15c/0x468
[   47.092643]  __alloc_skb+0xa4/0x498
[   47.096142]  igmpv3_newpack+0x158/0xd78
[   47.099987]  add_grhead+0x210/0x288
[   47.103485]  add_grec+0x6b0/0xb70
[   47.106811]  igmpv3_send_cr+0x2e0/0x6b8
[   47.110657]  igmp_ifc_timer_expire+0x4c/0x118
[   47.115027]  call_timer_fn+0x1cc/0xbe8
[   47.118785]  __run_timers+0x4d8/0xb28
[   47.122457]  run_timer_softirq+0x24/0x40
[   47.126389]  __do_softirq+0x2c0/0x117c
[   47.130142] 
[   47.131643] Freed by task 180:
[   47.134712]  save_stack+0x24/0xb0
[   47.138041]  __kasan_slab_free+0x108/0x180
[   47.142146]  kasan_slab_free+0x10/0x18
[   47.145904]  slab_free_freelist_hook+0xa4/0x1b0
[   47.150444]  kmem_cache_free+0x8c/0x528
[   47.154292]  kfree_skbmem+0x94/0x108
[   47.157880]  consume_skb+0x10c/0x5a8
[   47.161466]  __dev_kfree_skb_any+0x88/0xa0
[   47.165598]  brcmu_pkt_buf_free_skb+0x44/0x68 [brcmutil]
[   47.171023]  brcmf_txfinalize+0xec/0x190 [brcmfmac]
[   47.176016]  brcmf_proto_bcdc_txcomplete+0x1c0/0x210 [brcmfmac]
[   47.182056]  brcmf_sdio_sendfromq+0x8dc/0x1e80 [brcmfmac]
[   47.187568]  brcmf_sdio_dpc+0xb48/0x2108 [brcmfmac]
[   47.192529]  brcmf_sdio_dataworker+0xc8/0x238 [brcmfmac]
[   47.197859]  process_one_work+0x7fc/0x1a80
[   47.201965]  worker_thread+0x31c/0xc40
[   47.205726]  kthread+0x2d8/0x370
[   47.208967]  ret_from_fork+0x10/0x18
[   47.212546] 
[   47.214051] The buggy address belongs to the object at ffffff803f588280
[   47.214051]  which belongs to the cache skbuff_head_cache of size 208
[   47.227086] The buggy address is located 104 bytes inside of
[   47.227086]  208-byte region [ffffff803f588280, ffffff803f588350)
[   47.238814] The buggy address belongs to the page:
[   47.243618] page:ffffffff00dd6200 refcount:1 mapcount:0 mapping:ffffff804b6bf800 index:0xffffff803f589900 compound_mapcount: 0
[   47.255007] flags: 0x10200(slab|head)
[   47.258689] raw: 0000000000010200 ffffffff00dfa980 0000000200000002 ffffff804b6bf800
[   47.266439] raw: ffffff803f589900 0000000080190018 00000001ffffffff 0000000000000000
[   47.274180] page dumped because: kasan: bad access detected
[   47.279752] 
[   47.281251] Memory state around the buggy address:
[   47.286051]  ffffff803f588180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[   47.293277]  ffffff803f588200: fb fb fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[   47.300502] >ffffff803f588280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[   47.307723]                                                           ^
[   47.314343]  ffffff803f588300: fb fb fb fb fb fb fb fb fb fb fc fc fc fc fc fc
[   47.321569]  ffffff803f588380: fc fc fc fc fc fc fc fc fb fb fb fb fb fb fb fb
[   47.328789] ==================================================================




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

* [PATCH] brcmfmac: fix use-after-free bug
  2022-08-02 17:28 brcmfmac: fix use-after-free bug Alexander Coffin
@ 2022-08-02 17:28 ` Alexander Coffin
  2022-08-08  8:42   ` Arend Van Spriel
  0 siblings, 1 reply; 9+ messages in thread
From: Alexander Coffin @ 2022-08-02 17:28 UTC (permalink / raw)
  To: Arend van Spriel, Franky Lin, Hante Meuleman
  Cc: linux-wireless, brcm80211-dev-list.pdl, SHA-cyfmac-dev-list,
	Alexander Coffin

Signed-off-by: Alexander Coffin <alex.coffin@matician.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index 87aef211b35f..12ee8b7163fd 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -296,6 +296,7 @@ static netdev_tx_t brcmf_netdev_start_xmit(struct sk_buff *skb,
 	struct brcmf_pub *drvr = ifp->drvr;
 	struct ethhdr *eh;
 	int head_delta;
+	unsigned int tx_bytes = skb->len;
 
 	brcmf_dbg(DATA, "Enter, bsscfgidx=%d\n", ifp->bsscfgidx);
 
@@ -370,7 +371,7 @@ static netdev_tx_t brcmf_netdev_start_xmit(struct sk_buff *skb,
 		ndev->stats.tx_dropped++;
 	} else {
 		ndev->stats.tx_packets++;
-		ndev->stats.tx_bytes += skb->len;
+		ndev->stats.tx_bytes += tx_bytes;
 	}
 
 	/* Return ok: we always eat the packet */
-- 
2.30.2


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

* Re: [PATCH] brcmfmac: fix use-after-free bug
  2022-08-02 17:28 ` [PATCH] " Alexander Coffin
@ 2022-08-08  8:42   ` Arend Van Spriel
  2022-08-08  9:38     ` Alexander Coffin
  0 siblings, 1 reply; 9+ messages in thread
From: Arend Van Spriel @ 2022-08-08  8:42 UTC (permalink / raw)
  To: Alexander Coffin, Franky Lin, Hante Meuleman
  Cc: linux-wireless, brcm80211-dev-list.pdl, SHA-cyfmac-dev-list

On 8/2/2022 7:28 PM, Alexander Coffin wrote:
 >

A commit message would have been nice...

> Signed-off-by: Alexander Coffin <alex.coffin@matician.com>
> ---
>   drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
> index 87aef211b35f..12ee8b7163fd 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
> @@ -296,6 +296,7 @@ static netdev_tx_t brcmf_netdev_start_xmit(struct sk_buff *skb,
>   	struct brcmf_pub *drvr = ifp->drvr;
>   	struct ethhdr *eh;
>   	int head_delta;
> +	unsigned int tx_bytes = skb->len;
>   
>   	brcmf_dbg(DATA, "Enter, bsscfgidx=%d\n", ifp->bsscfgidx);
>   
> @@ -370,7 +371,7 @@ static netdev_tx_t brcmf_netdev_start_xmit(struct sk_buff *skb,
>   		ndev->stats.tx_dropped++;
>   	} else {
>   		ndev->stats.tx_packets++;
> -		ndev->stats.tx_bytes += skb->len;
> +		ndev->stats.tx_bytes += tx_bytes;

Why would this be a use-after-free? We only get here when ret is zero. 
In that case the skb is not freed. If there would be a commit message 
with some error report that proofs there is a use-after-free I would 
look into this further, but now I just say NAK.

Regards,
Arend

>   	}
>   
>   	/* Return ok: we always eat the packet */

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

* Re: [PATCH] brcmfmac: fix use-after-free bug
  2022-08-08  8:42   ` Arend Van Spriel
@ 2022-08-08  9:38     ` Alexander Coffin
  2022-08-08 12:10       ` Arend Van Spriel
  0 siblings, 1 reply; 9+ messages in thread
From: Alexander Coffin @ 2022-08-08  9:38 UTC (permalink / raw)
  To: Arend Van Spriel
  Cc: Franky Lin, Hante Meuleman, linux-wireless,
	brcm80211-dev-list.pdl, SHA-cyfmac-dev-list

I am resending this message as apparently it wasn't delivered to some
people as it was an HTML message. I apologize for the double email,
but I forgot to tell Gmail to use plain text only.

Arend,

> A commit message would have been nice...

> If there would be a commit message with some error report that proofs there is a use-after-free

I apologize for not including a longer commit message. I thought that
my stack trace in my first email would be sufficient, but looking back
I see how I should have clarified what was going wrong. What occurs is
that line 360 of core.c

> ret = brcmf_proto_tx_queue_data(drvr, ifp->ifidx, skb);

may be entirely completed (as in not only scheduled, but also the
entire transaction may have completed) by the time that `skb->len` is
invoked which means that skb will have been freed by the corresponding
later function (in this case brcmu_pkt_buf_free_skb if you see the
trace from my first email).

> We only get here when ret is zero.

Therefore this error only occurs when ret is zero, but skb may have
been freed after line 360, and before that line (369) if how the
kernel schedules tasks is very unfavorable.

> ndev->stats.tx_bytes += skb->len;

Please let me know if you need any further information.

Sorry,
Alexander Coffin

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

* Re: [PATCH] brcmfmac: fix use-after-free bug
  2022-08-08  9:38     ` Alexander Coffin
@ 2022-08-08 12:10       ` Arend Van Spriel
  2022-08-08 17:49         ` [PATCH v2] " Alexander Coffin
  0 siblings, 1 reply; 9+ messages in thread
From: Arend Van Spriel @ 2022-08-08 12:10 UTC (permalink / raw)
  To: Alexander Coffin
  Cc: Franky Lin, Hante Meuleman, linux-wireless,
	brcm80211-dev-list.pdl, SHA-cyfmac-dev-list

On 8/8/2022 11:38 AM, Alexander Coffin wrote:
> I am resending this message as apparently it wasn't delivered to some
> people as it was an HTML message. I apologize for the double email,
> but I forgot to tell Gmail to use plain text only.
> 
> Arend,
> 
>> A commit message would have been nice...
> 
>> If there would be a commit message with some error report that proofs there is a use-after-free
> 
> I apologize for not including a longer commit message. I thought that
> my stack trace in my first email would be sufficient, but looking back
> I see how I should have clarified what was going wrong. What occurs is
> that line 360 of core.c
> 
>> ret = brcmf_proto_tx_queue_data(drvr, ifp->ifidx, skb);
> 
> may be entirely completed (as in not only scheduled, but also the
> entire transaction may have completed) by the time that `skb->len` is
> invoked which means that skb will have been freed by the corresponding
> later function (in this case brcmu_pkt_buf_free_skb if you see the
> trace from my first email).
> 
>> We only get here when ret is zero.
> 
> Therefore this error only occurs when ret is zero, but skb may have
> been freed after line 360, and before that line (369) if how the
> kernel schedules tasks is very unfavorable.
> 
>> ndev->stats.tx_bytes += skb->len;
> 
> Please let me know if you need any further information.

Not sure I've seen your first email. It is better to have it all in the 
patch so it is clear the patch solves an actual observed failure. So if 
you can dig up that stack trace and add it to the patch I will be happy 
to acknowledge the patch so it can be taken upstream.

Regards,
Arend

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

* [PATCH v2] brcmfmac: fix use-after-free bug
  2022-08-08 12:10       ` Arend Van Spriel
@ 2022-08-08 17:49         ` Alexander Coffin
  2022-08-12 15:05           ` Alexander Coffin
  2022-09-07  7:57           ` [v2] wifi: brcmfmac: fix use-after-free bug in brcmf_netdev_start_xmit() Kalle Valo
  0 siblings, 2 replies; 9+ messages in thread
From: Alexander Coffin @ 2022-08-08 17:49 UTC (permalink / raw)
  To: Arend van Spriel, Franky Lin, Hante Meuleman
  Cc: linux-wireless, brcm80211-dev-list.pdl, SHA-cyfmac-dev-list,
	Alexander Coffin

> ret = brcmf_proto_tx_queue_data(drvr, ifp->ifidx, skb);

may be schedule, and then complete before the line

> ndev->stats.tx_bytes += skb->len;

[   46.912801] ==================================================================
[   46.920552] BUG: KASAN: use-after-free in brcmf_netdev_start_xmit+0x718/0x8c8 [brcmfmac]
[   46.928673] Read of size 4 at addr ffffff803f5882e8 by task systemd-resolve/328
[   46.935991]
[   46.937514] CPU: 1 PID: 328 Comm: systemd-resolve Tainted: G           O      5.4.199-[REDACTED] #1
[   46.947255] Hardware name: [REDACTED]
[   46.954568] Call trace:
[   46.957037]  dump_backtrace+0x0/0x2b8
[   46.960719]  show_stack+0x24/0x30
[   46.964052]  dump_stack+0x128/0x194
[   46.967557]  print_address_description.isra.0+0x64/0x380
[   46.972877]  __kasan_report+0x1d4/0x240
[   46.976723]  kasan_report+0xc/0x18
[   46.980138]  __asan_report_load4_noabort+0x18/0x20
[   46.985027]  brcmf_netdev_start_xmit+0x718/0x8c8 [brcmfmac]
[   46.990613]  dev_hard_start_xmit+0x1bc/0xda0
[   46.994894]  sch_direct_xmit+0x198/0xd08
[   46.998827]  __qdisc_run+0x37c/0x1dc0
[   47.002500]  __dev_queue_xmit+0x1528/0x21f8
[   47.006692]  dev_queue_xmit+0x24/0x30
[   47.010366]  neigh_resolve_output+0x37c/0x678
[   47.014734]  ip_finish_output2+0x598/0x2458
[   47.018927]  __ip_finish_output+0x300/0x730
[   47.023118]  ip_output+0x2e0/0x430
[   47.026530]  ip_local_out+0x90/0x140
[   47.030117]  igmpv3_sendpack+0x14c/0x228
[   47.034049]  igmpv3_send_cr+0x384/0x6b8
[   47.037895]  igmp_ifc_timer_expire+0x4c/0x118
[   47.042262]  call_timer_fn+0x1cc/0xbe8
[   47.046021]  __run_timers+0x4d8/0xb28
[   47.049693]  run_timer_softirq+0x24/0x40
[   47.053626]  __do_softirq+0x2c0/0x117c
[   47.057387]  irq_exit+0x2dc/0x388
[   47.060715]  __handle_domain_irq+0xb4/0x158
[   47.064908]  gic_handle_irq+0x58/0xb0
[   47.068581]  el0_irq_naked+0x50/0x5c
[   47.072162]
[   47.073665] Allocated by task 328:
[   47.077083]  save_stack+0x24/0xb0
[   47.080410]  __kasan_kmalloc.isra.0+0xc0/0xe0
[   47.084776]  kasan_slab_alloc+0x14/0x20
[   47.088622]  kmem_cache_alloc+0x15c/0x468
[   47.092643]  __alloc_skb+0xa4/0x498
[   47.096142]  igmpv3_newpack+0x158/0xd78
[   47.099987]  add_grhead+0x210/0x288
[   47.103485]  add_grec+0x6b0/0xb70
[   47.106811]  igmpv3_send_cr+0x2e0/0x6b8
[   47.110657]  igmp_ifc_timer_expire+0x4c/0x118
[   47.115027]  call_timer_fn+0x1cc/0xbe8
[   47.118785]  __run_timers+0x4d8/0xb28
[   47.122457]  run_timer_softirq+0x24/0x40
[   47.126389]  __do_softirq+0x2c0/0x117c
[   47.130142]
[   47.131643] Freed by task 180:
[   47.134712]  save_stack+0x24/0xb0
[   47.138041]  __kasan_slab_free+0x108/0x180
[   47.142146]  kasan_slab_free+0x10/0x18
[   47.145904]  slab_free_freelist_hook+0xa4/0x1b0
[   47.150444]  kmem_cache_free+0x8c/0x528
[   47.154292]  kfree_skbmem+0x94/0x108
[   47.157880]  consume_skb+0x10c/0x5a8
[   47.161466]  __dev_kfree_skb_any+0x88/0xa0
[   47.165598]  brcmu_pkt_buf_free_skb+0x44/0x68 [brcmutil]
[   47.171023]  brcmf_txfinalize+0xec/0x190 [brcmfmac]
[   47.176016]  brcmf_proto_bcdc_txcomplete+0x1c0/0x210 [brcmfmac]
[   47.182056]  brcmf_sdio_sendfromq+0x8dc/0x1e80 [brcmfmac]
[   47.187568]  brcmf_sdio_dpc+0xb48/0x2108 [brcmfmac]
[   47.192529]  brcmf_sdio_dataworker+0xc8/0x238 [brcmfmac]
[   47.197859]  process_one_work+0x7fc/0x1a80
[   47.201965]  worker_thread+0x31c/0xc40
[   47.205726]  kthread+0x2d8/0x370
[   47.208967]  ret_from_fork+0x10/0x18
[   47.212546]
[   47.214051] The buggy address belongs to the object at ffffff803f588280
[   47.214051]  which belongs to the cache skbuff_head_cache of size 208
[   47.227086] The buggy address is located 104 bytes inside of
[   47.227086]  208-byte region [ffffff803f588280, ffffff803f588350)
[   47.238814] The buggy address belongs to the page:
[   47.243618] page:ffffffff00dd6200 refcount:1 mapcount:0 mapping:ffffff804b6bf800 index:0xffffff803f589900 compound_mapcount: 0
[   47.255007] flags: 0x10200(slab|head)
[   47.258689] raw: 0000000000010200 ffffffff00dfa980 0000000200000002 ffffff804b6bf800
[   47.266439] raw: ffffff803f589900 0000000080190018 00000001ffffffff 0000000000000000
[   47.274180] page dumped because: kasan: bad access detected
[   47.279752]
[   47.281251] Memory state around the buggy address:
[   47.286051]  ffffff803f588180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[   47.293277]  ffffff803f588200: fb fb fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[   47.300502] >ffffff803f588280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[   47.307723]                                                           ^
[   47.314343]  ffffff803f588300: fb fb fb fb fb fb fb fb fb fb fc fc fc fc fc fc
[   47.321569]  ffffff803f588380: fc fc fc fc fc fc fc fc fb fb fb fb fb fb fb fb
[   47.328789] ==================================================================

Signed-off-by: Alexander Coffin <alex.coffin@matician.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index 87aef211b35f..12ee8b7163fd 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -296,6 +296,7 @@ static netdev_tx_t brcmf_netdev_start_xmit(struct sk_buff *skb,
 	struct brcmf_pub *drvr = ifp->drvr;
 	struct ethhdr *eh;
 	int head_delta;
+	unsigned int tx_bytes = skb->len;
 
 	brcmf_dbg(DATA, "Enter, bsscfgidx=%d\n", ifp->bsscfgidx);
 
@@ -370,7 +371,7 @@ static netdev_tx_t brcmf_netdev_start_xmit(struct sk_buff *skb,
 		ndev->stats.tx_dropped++;
 	} else {
 		ndev->stats.tx_packets++;
-		ndev->stats.tx_bytes += skb->len;
+		ndev->stats.tx_bytes += tx_bytes;
 	}
 
 	/* Return ok: we always eat the packet */
-- 
2.30.2


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

* Re: [PATCH v2] brcmfmac: fix use-after-free bug
  2022-08-08 17:49         ` [PATCH v2] " Alexander Coffin
@ 2022-08-12 15:05           ` Alexander Coffin
  2022-08-13 18:24             ` Alexander Coffin
  2022-09-07  7:57           ` [v2] wifi: brcmfmac: fix use-after-free bug in brcmf_netdev_start_xmit() Kalle Valo
  1 sibling, 1 reply; 9+ messages in thread
From: Alexander Coffin @ 2022-08-12 15:05 UTC (permalink / raw)
  To: Arend van Spriel, Franky Lin, Hante Meuleman
  Cc: linux-wireless, brcm80211-dev-list.pdl, SHA-cyfmac-dev-list

Arend,

I just wanted to double check that you have received my second patch
as I'm beginning to be slightly worried that your spam filters are
blocking my stack traces.

- Alexander Coffin

On Mon, Aug 8, 2022 at 10:50 AM Alexander Coffin
<alex.coffin@matician.com> wrote:
>
> > ret = brcmf_proto_tx_queue_data(drvr, ifp->ifidx, skb);
>
> may be schedule, and then complete before the line
>
> > ndev->stats.tx_bytes += skb->len;
>
> [   46.912801] ==================================================================
> [   46.920552] BUG: KASAN: use-after-free in brcmf_netdev_start_xmit+0x718/0x8c8 [brcmfmac]
> [   46.928673] Read of size 4 at addr ffffff803f5882e8 by task systemd-resolve/328
> [   46.935991]
> [   46.937514] CPU: 1 PID: 328 Comm: systemd-resolve Tainted: G           O      5.4.199-[REDACTED] #1
> [   46.947255] Hardware name: [REDACTED]
> [   46.954568] Call trace:
> [   46.957037]  dump_backtrace+0x0/0x2b8
> [   46.960719]  show_stack+0x24/0x30
> [   46.964052]  dump_stack+0x128/0x194
> [   46.967557]  print_address_description.isra.0+0x64/0x380
> [   46.972877]  __kasan_report+0x1d4/0x240
> [   46.976723]  kasan_report+0xc/0x18
> [   46.980138]  __asan_report_load4_noabort+0x18/0x20
> [   46.985027]  brcmf_netdev_start_xmit+0x718/0x8c8 [brcmfmac]
> [   46.990613]  dev_hard_start_xmit+0x1bc/0xda0
> [   46.994894]  sch_direct_xmit+0x198/0xd08
> [   46.998827]  __qdisc_run+0x37c/0x1dc0
> [   47.002500]  __dev_queue_xmit+0x1528/0x21f8
> [   47.006692]  dev_queue_xmit+0x24/0x30
> [   47.010366]  neigh_resolve_output+0x37c/0x678
> [   47.014734]  ip_finish_output2+0x598/0x2458
> [   47.018927]  __ip_finish_output+0x300/0x730
> [   47.023118]  ip_output+0x2e0/0x430
> [   47.026530]  ip_local_out+0x90/0x140
> [   47.030117]  igmpv3_sendpack+0x14c/0x228
> [   47.034049]  igmpv3_send_cr+0x384/0x6b8
> [   47.037895]  igmp_ifc_timer_expire+0x4c/0x118
> [   47.042262]  call_timer_fn+0x1cc/0xbe8
> [   47.046021]  __run_timers+0x4d8/0xb28
> [   47.049693]  run_timer_softirq+0x24/0x40
> [   47.053626]  __do_softirq+0x2c0/0x117c
> [   47.057387]  irq_exit+0x2dc/0x388
> [   47.060715]  __handle_domain_irq+0xb4/0x158
> [   47.064908]  gic_handle_irq+0x58/0xb0
> [   47.068581]  el0_irq_naked+0x50/0x5c
> [   47.072162]
> [   47.073665] Allocated by task 328:
> [   47.077083]  save_stack+0x24/0xb0
> [   47.080410]  __kasan_kmalloc.isra.0+0xc0/0xe0
> [   47.084776]  kasan_slab_alloc+0x14/0x20
> [   47.088622]  kmem_cache_alloc+0x15c/0x468
> [   47.092643]  __alloc_skb+0xa4/0x498
> [   47.096142]  igmpv3_newpack+0x158/0xd78
> [   47.099987]  add_grhead+0x210/0x288
> [   47.103485]  add_grec+0x6b0/0xb70
> [   47.106811]  igmpv3_send_cr+0x2e0/0x6b8
> [   47.110657]  igmp_ifc_timer_expire+0x4c/0x118
> [   47.115027]  call_timer_fn+0x1cc/0xbe8
> [   47.118785]  __run_timers+0x4d8/0xb28
> [   47.122457]  run_timer_softirq+0x24/0x40
> [   47.126389]  __do_softirq+0x2c0/0x117c
> [   47.130142]
> [   47.131643] Freed by task 180:
> [   47.134712]  save_stack+0x24/0xb0
> [   47.138041]  __kasan_slab_free+0x108/0x180
> [   47.142146]  kasan_slab_free+0x10/0x18
> [   47.145904]  slab_free_freelist_hook+0xa4/0x1b0
> [   47.150444]  kmem_cache_free+0x8c/0x528
> [   47.154292]  kfree_skbmem+0x94/0x108
> [   47.157880]  consume_skb+0x10c/0x5a8
> [   47.161466]  __dev_kfree_skb_any+0x88/0xa0
> [   47.165598]  brcmu_pkt_buf_free_skb+0x44/0x68 [brcmutil]
> [   47.171023]  brcmf_txfinalize+0xec/0x190 [brcmfmac]
> [   47.176016]  brcmf_proto_bcdc_txcomplete+0x1c0/0x210 [brcmfmac]
> [   47.182056]  brcmf_sdio_sendfromq+0x8dc/0x1e80 [brcmfmac]
> [   47.187568]  brcmf_sdio_dpc+0xb48/0x2108 [brcmfmac]
> [   47.192529]  brcmf_sdio_dataworker+0xc8/0x238 [brcmfmac]
> [   47.197859]  process_one_work+0x7fc/0x1a80
> [   47.201965]  worker_thread+0x31c/0xc40
> [   47.205726]  kthread+0x2d8/0x370
> [   47.208967]  ret_from_fork+0x10/0x18
> [   47.212546]
> [   47.214051] The buggy address belongs to the object at ffffff803f588280
> [   47.214051]  which belongs to the cache skbuff_head_cache of size 208
> [   47.227086] The buggy address is located 104 bytes inside of
> [   47.227086]  208-byte region [ffffff803f588280, ffffff803f588350)
> [   47.238814] The buggy address belongs to the page:
> [   47.243618] page:ffffffff00dd6200 refcount:1 mapcount:0 mapping:ffffff804b6bf800 index:0xffffff803f589900 compound_mapcount: 0
> [   47.255007] flags: 0x10200(slab|head)
> [   47.258689] raw: 0000000000010200 ffffffff00dfa980 0000000200000002 ffffff804b6bf800
> [   47.266439] raw: ffffff803f589900 0000000080190018 00000001ffffffff 0000000000000000
> [   47.274180] page dumped because: kasan: bad access detected
> [   47.279752]
> [   47.281251] Memory state around the buggy address:
> [   47.286051]  ffffff803f588180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [   47.293277]  ffffff803f588200: fb fb fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> [   47.300502] >ffffff803f588280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [   47.307723]                                                           ^
> [   47.314343]  ffffff803f588300: fb fb fb fb fb fb fb fb fb fb fc fc fc fc fc fc
> [   47.321569]  ffffff803f588380: fc fc fc fc fc fc fc fc fb fb fb fb fb fb fb fb
> [   47.328789] ==================================================================
>
> Signed-off-by: Alexander Coffin <alex.coffin@matician.com>
> ---
>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
> index 87aef211b35f..12ee8b7163fd 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
> @@ -296,6 +296,7 @@ static netdev_tx_t brcmf_netdev_start_xmit(struct sk_buff *skb,
>         struct brcmf_pub *drvr = ifp->drvr;
>         struct ethhdr *eh;
>         int head_delta;
> +       unsigned int tx_bytes = skb->len;
>
>         brcmf_dbg(DATA, "Enter, bsscfgidx=%d\n", ifp->bsscfgidx);
>
> @@ -370,7 +371,7 @@ static netdev_tx_t brcmf_netdev_start_xmit(struct sk_buff *skb,
>                 ndev->stats.tx_dropped++;
>         } else {
>                 ndev->stats.tx_packets++;
> -               ndev->stats.tx_bytes += skb->len;
> +               ndev->stats.tx_bytes += tx_bytes;
>         }
>
>         /* Return ok: we always eat the packet */
> --
> 2.30.2
>

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

* Re: [PATCH v2] brcmfmac: fix use-after-free bug
  2022-08-12 15:05           ` Alexander Coffin
@ 2022-08-13 18:24             ` Alexander Coffin
  0 siblings, 0 replies; 9+ messages in thread
From: Alexander Coffin @ 2022-08-13 18:24 UTC (permalink / raw)
  To: Arend van Spriel, Franky Lin, Hante Meuleman
  Cc: linux-wireless, brcm80211-dev-list.pdl, SHA-cyfmac-dev-list

Arend,

I just wanted to double check that you have received my second patch
as I'm beginning to be slightly worried that your spam filters are
blocking my stack traces.

- Alexander Coffin

I'm resending this message as I realized that Gmail automatically
quoting the stack trace would have made it always go to your spam if I
was correct.

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

* Re: [v2] wifi: brcmfmac: fix use-after-free bug in brcmf_netdev_start_xmit()
  2022-08-08 17:49         ` [PATCH v2] " Alexander Coffin
  2022-08-12 15:05           ` Alexander Coffin
@ 2022-09-07  7:57           ` Kalle Valo
  1 sibling, 0 replies; 9+ messages in thread
From: Kalle Valo @ 2022-09-07  7:57 UTC (permalink / raw)
  To: Alexander Coffin
  Cc: Arend van Spriel, Franky Lin, Hante Meuleman, linux-wireless,
	brcm80211-dev-list.pdl, SHA-cyfmac-dev-list, Alexander Coffin

Alexander Coffin <alex.coffin@matician.com> wrote:

> > ret = brcmf_proto_tx_queue_data(drvr, ifp->ifidx, skb);
> 
> may be schedule, and then complete before the line
> 
> > ndev->stats.tx_bytes += skb->len;
> 
> [   46.912801] ==================================================================
> [   46.920552] BUG: KASAN: use-after-free in brcmf_netdev_start_xmit+0x718/0x8c8 [brcmfmac]
> [   46.928673] Read of size 4 at addr ffffff803f5882e8 by task systemd-resolve/328
> [   46.935991]
> [   46.937514] CPU: 1 PID: 328 Comm: systemd-resolve Tainted: G           O      5.4.199-[REDACTED] #1
> [   46.947255] Hardware name: [REDACTED]
> [   46.954568] Call trace:
> [   46.957037]  dump_backtrace+0x0/0x2b8
> [   46.960719]  show_stack+0x24/0x30
> [   46.964052]  dump_stack+0x128/0x194
> [   46.967557]  print_address_description.isra.0+0x64/0x380
> [   46.972877]  __kasan_report+0x1d4/0x240
> [   46.976723]  kasan_report+0xc/0x18
> [   46.980138]  __asan_report_load4_noabort+0x18/0x20
> [   46.985027]  brcmf_netdev_start_xmit+0x718/0x8c8 [brcmfmac]
> [   46.990613]  dev_hard_start_xmit+0x1bc/0xda0
> [   46.994894]  sch_direct_xmit+0x198/0xd08
> [   46.998827]  __qdisc_run+0x37c/0x1dc0
> [   47.002500]  __dev_queue_xmit+0x1528/0x21f8
> [   47.006692]  dev_queue_xmit+0x24/0x30
> [   47.010366]  neigh_resolve_output+0x37c/0x678
> [   47.014734]  ip_finish_output2+0x598/0x2458
> [   47.018927]  __ip_finish_output+0x300/0x730
> [   47.023118]  ip_output+0x2e0/0x430
> [   47.026530]  ip_local_out+0x90/0x140
> [   47.030117]  igmpv3_sendpack+0x14c/0x228
> [   47.034049]  igmpv3_send_cr+0x384/0x6b8
> [   47.037895]  igmp_ifc_timer_expire+0x4c/0x118
> [   47.042262]  call_timer_fn+0x1cc/0xbe8
> [   47.046021]  __run_timers+0x4d8/0xb28
> [   47.049693]  run_timer_softirq+0x24/0x40
> [   47.053626]  __do_softirq+0x2c0/0x117c
> [   47.057387]  irq_exit+0x2dc/0x388
> [   47.060715]  __handle_domain_irq+0xb4/0x158
> [   47.064908]  gic_handle_irq+0x58/0xb0
> [   47.068581]  el0_irq_naked+0x50/0x5c
> [   47.072162]
> [   47.073665] Allocated by task 328:
> [   47.077083]  save_stack+0x24/0xb0
> [   47.080410]  __kasan_kmalloc.isra.0+0xc0/0xe0
> [   47.084776]  kasan_slab_alloc+0x14/0x20
> [   47.088622]  kmem_cache_alloc+0x15c/0x468
> [   47.092643]  __alloc_skb+0xa4/0x498
> [   47.096142]  igmpv3_newpack+0x158/0xd78
> [   47.099987]  add_grhead+0x210/0x288
> [   47.103485]  add_grec+0x6b0/0xb70
> [   47.106811]  igmpv3_send_cr+0x2e0/0x6b8
> [   47.110657]  igmp_ifc_timer_expire+0x4c/0x118
> [   47.115027]  call_timer_fn+0x1cc/0xbe8
> [   47.118785]  __run_timers+0x4d8/0xb28
> [   47.122457]  run_timer_softirq+0x24/0x40
> [   47.126389]  __do_softirq+0x2c0/0x117c
> [   47.130142]
> [   47.131643] Freed by task 180:
> [   47.134712]  save_stack+0x24/0xb0
> [   47.138041]  __kasan_slab_free+0x108/0x180
> [   47.142146]  kasan_slab_free+0x10/0x18
> [   47.145904]  slab_free_freelist_hook+0xa4/0x1b0
> [   47.150444]  kmem_cache_free+0x8c/0x528
> [   47.154292]  kfree_skbmem+0x94/0x108
> [   47.157880]  consume_skb+0x10c/0x5a8
> [   47.161466]  __dev_kfree_skb_any+0x88/0xa0
> [   47.165598]  brcmu_pkt_buf_free_skb+0x44/0x68 [brcmutil]
> [   47.171023]  brcmf_txfinalize+0xec/0x190 [brcmfmac]
> [   47.176016]  brcmf_proto_bcdc_txcomplete+0x1c0/0x210 [brcmfmac]
> [   47.182056]  brcmf_sdio_sendfromq+0x8dc/0x1e80 [brcmfmac]
> [   47.187568]  brcmf_sdio_dpc+0xb48/0x2108 [brcmfmac]
> [   47.192529]  brcmf_sdio_dataworker+0xc8/0x238 [brcmfmac]
> [   47.197859]  process_one_work+0x7fc/0x1a80
> [   47.201965]  worker_thread+0x31c/0xc40
> [   47.205726]  kthread+0x2d8/0x370
> [   47.208967]  ret_from_fork+0x10/0x18
> [   47.212546]
> [   47.214051] The buggy address belongs to the object at ffffff803f588280
> [   47.214051]  which belongs to the cache skbuff_head_cache of size 208
> [   47.227086] The buggy address is located 104 bytes inside of
> [   47.227086]  208-byte region [ffffff803f588280, ffffff803f588350)
> [   47.238814] The buggy address belongs to the page:
> [   47.243618] page:ffffffff00dd6200 refcount:1 mapcount:0 mapping:ffffff804b6bf800 index:0xffffff803f589900 compound_mapcount: 0
> [   47.255007] flags: 0x10200(slab|head)
> [   47.258689] raw: 0000000000010200 ffffffff00dfa980 0000000200000002 ffffff804b6bf800
> [   47.266439] raw: ffffff803f589900 0000000080190018 00000001ffffffff 0000000000000000
> [   47.274180] page dumped because: kasan: bad access detected
> [   47.279752]
> [   47.281251] Memory state around the buggy address:
> [   47.286051]  ffffff803f588180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [   47.293277]  ffffff803f588200: fb fb fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> [   47.300502] >ffffff803f588280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [   47.307723]                                                           ^
> [   47.314343]  ffffff803f588300: fb fb fb fb fb fb fb fb fb fb fc fc fc fc fc fc
> [   47.321569]  ffffff803f588380: fc fc fc fc fc fc fc fc fb fb fb fb fb fb fb fb
> [   47.328789] ==================================================================
> 
> Signed-off-by: Alexander Coffin <alex.coffin@matician.com>

Patch applied to wireless-next.git, thanks.

3f42faf6db43 wifi: brcmfmac: fix use-after-free bug in brcmf_netdev_start_xmit()

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20220808174925.3922558-1-alex.coffin@matician.com/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


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

end of thread, other threads:[~2022-09-07  7:57 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-02 17:28 brcmfmac: fix use-after-free bug Alexander Coffin
2022-08-02 17:28 ` [PATCH] " Alexander Coffin
2022-08-08  8:42   ` Arend Van Spriel
2022-08-08  9:38     ` Alexander Coffin
2022-08-08 12:10       ` Arend Van Spriel
2022-08-08 17:49         ` [PATCH v2] " Alexander Coffin
2022-08-12 15:05           ` Alexander Coffin
2022-08-13 18:24             ` Alexander Coffin
2022-09-07  7:57           ` [v2] wifi: brcmfmac: fix use-after-free bug in brcmf_netdev_start_xmit() Kalle Valo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.