[PATCH net V2] macsec: Fix use-after-free while sending the offloading packet

[PATCH net V2] macsec: Fix use-after-free while sending the offloading packet

[Tariq Toukan]

From: Jianbo Liu <jianbol@nvidia.com>

KASAN reports the following UAF. The metadata_dst, which is used to
store the SCI value for macsec offload, is already freed by
metadata_dst_free() in macsec_free_netdev(), while driver still use it
for sending the packet.

To fix this issue, dst_release() is used instead to release
metadata_dst. So it is not freed instantly in macsec_free_netdev() if
still referenced by skb.

 BUG: KASAN: slab-use-after-free in mlx5e_xmit+0x1e8f/0x4190 [mlx5_core]
 Read of size 2 at addr ffff88813e42e038 by task kworker/7:2/714
 [...]
 Workqueue: mld mld_ifc_work
 Call Trace:
  <TASK>
  dump_stack_lvl+0x51/0x60
  print_report+0xc1/0x600
  kasan_report+0xab/0xe0
  mlx5e_xmit+0x1e8f/0x4190 [mlx5_core]
  dev_hard_start_xmit+0x120/0x530
  sch_direct_xmit+0x149/0x11e0
  __qdisc_run+0x3ad/0x1730
  __dev_queue_xmit+0x1196/0x2ed0
  vlan_dev_hard_start_xmit+0x32e/0x510 [8021q]
  dev_hard_start_xmit+0x120/0x530
  __dev_queue_xmit+0x14a7/0x2ed0
  macsec_start_xmit+0x13e9/0x2340
  dev_hard_start_xmit+0x120/0x530
  __dev_queue_xmit+0x14a7/0x2ed0
  ip6_finish_output2+0x923/0x1a70
  ip6_finish_output+0x2d7/0x970
  ip6_output+0x1ce/0x3a0
  NF_HOOK.constprop.0+0x15f/0x190
  mld_sendpack+0x59a/0xbd0
  mld_ifc_work+0x48a/0xa80
  process_one_work+0x5aa/0xe50
  worker_thread+0x79c/0x1290
  kthread+0x28f/0x350
  ret_from_fork+0x2d/0x70
  ret_from_fork_asm+0x11/0x20
  </TASK>

 Allocated by task 3922:
  kasan_save_stack+0x20/0x40
  kasan_save_track+0x10/0x30
  __kasan_kmalloc+0x77/0x90
  __kmalloc_noprof+0x188/0x400
  metadata_dst_alloc+0x1f/0x4e0
  macsec_newlink+0x914/0x1410
  __rtnl_newlink+0xe08/0x15b0
  rtnl_newlink+0x5f/0x90
  rtnetlink_rcv_msg+0x667/0xa80
  netlink_rcv_skb+0x12c/0x360
  netlink_unicast+0x551/0x770
  netlink_sendmsg+0x72d/0xbd0
  __sock_sendmsg+0xc5/0x190
  ____sys_sendmsg+0x52e/0x6a0
  ___sys_sendmsg+0xeb/0x170
  __sys_sendmsg+0xb5/0x140
  do_syscall_64+0x4c/0x100
  entry_SYSCALL_64_after_hwframe+0x4b/0x53

 Freed by task 4011:
  kasan_save_stack+0x20/0x40
  kasan_save_track+0x10/0x30
  kasan_save_free_info+0x37/0x50
  poison_slab_object+0x10c/0x190
  __kasan_slab_free+0x11/0x30
  kfree+0xe0/0x290
  macsec_free_netdev+0x3f/0x140
  netdev_run_todo+0x450/0xc70
  rtnetlink_rcv_msg+0x66f/0xa80
  netlink_rcv_skb+0x12c/0x360
  netlink_unicast+0x551/0x770
  netlink_sendmsg+0x72d/0xbd0
  __sock_sendmsg+0xc5/0x190
  ____sys_sendmsg+0x52e/0x6a0
  ___sys_sendmsg+0xeb/0x170
  __sys_sendmsg+0xb5/0x140
  do_syscall_64+0x4c/0x100
  entry_SYSCALL_64_after_hwframe+0x4b/0x53

Fixes: 0a28bfd4971f ("net/macsec: Add MACsec skb_metadata_dst Tx Data path support")
Signed-off-by: Jianbo Liu <jianbol@nvidia.com>
Reviewed-by: Patrisious Haddad <phaddad@nvidia.com>
Reviewed-by: Chris Mi <cmi@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 drivers/net/macsec.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

V2:
- Removed NULL check before call to dst_release().

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index 12d1b205f6d1..6223c1fa2f09 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -3816,8 +3816,7 @@ static void macsec_free_netdev(struct net_device *dev)
 {
 	struct macsec_dev *macsec = macsec_priv(dev);
 
-	if (macsec->secy.tx_sc.md_dst)
-		metadata_dst_free(macsec->secy.tx_sc.md_dst);
+	dst_release(&macsec->secy.tx_sc.md_dst->dst);
 	free_percpu(macsec->stats);
 	free_percpu(macsec->secy.tx_sc.stats);
 
-- 
2.44.0

Re: [PATCH net V2] macsec: Fix use-after-free while sending the offloading packet

[Simon Horman]

On Mon, Oct 21, 2024 at 01:03:09PM +0300, Tariq Toukan wrote:
> From: Jianbo Liu <jianbol@nvidia.com>
> 
> KASAN reports the following UAF. The metadata_dst, which is used to
> store the SCI value for macsec offload, is already freed by
> metadata_dst_free() in macsec_free_netdev(), while driver still use it
> for sending the packet.
> 
> To fix this issue, dst_release() is used instead to release
> metadata_dst. So it is not freed instantly in macsec_free_netdev() if
> still referenced by skb.
> 
>  BUG: KASAN: slab-use-after-free in mlx5e_xmit+0x1e8f/0x4190 [mlx5_core]
>  Read of size 2 at addr ffff88813e42e038 by task kworker/7:2/714
>  [...]

...

> Fixes: 0a28bfd4971f ("net/macsec: Add MACsec skb_metadata_dst Tx Data path support")
> Signed-off-by: Jianbo Liu <jianbol@nvidia.com>
> Reviewed-by: Patrisious Haddad <phaddad@nvidia.com>
> Reviewed-by: Chris Mi <cmi@nvidia.com>
> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
> ---
>  drivers/net/macsec.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> V2:
> - Removed NULL check before call to dst_release().

Thanks Jianbo, all, for the update.

Reviewed-by: Simon Horman <horms@kernel.org>

Please do follow up on the unencrypted packet issue flagged
by Sabrina in her review of v1 [1].

https://lore.kernel.org/all/Zw6CntwUyqM6CivS@hog/

...

Re: [PATCH net V2] macsec: Fix use-after-free while sending the offloading packet

[Sabrina Dubroca]

2024-10-21, 13:03:09 +0300, Tariq Toukan wrote:
> From: Jianbo Liu <jianbol@nvidia.com>
> 
> KASAN reports the following UAF. The metadata_dst, which is used to
> store the SCI value for macsec offload, is already freed by
> metadata_dst_free() in macsec_free_netdev(), while driver still use it
> for sending the packet.
> 
> To fix this issue, dst_release() is used instead to release
> metadata_dst. So it is not freed instantly in macsec_free_netdev() if
> still referenced by skb.

Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>

-- 
Sabrina

Re: [PATCH net V2] macsec: Fix use-after-free while sending the offloading packet

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon, 21 Oct 2024 13:03:09 +0300 you wrote:
> From: Jianbo Liu <jianbol@nvidia.com>
> 
> KASAN reports the following UAF. The metadata_dst, which is used to
> store the SCI value for macsec offload, is already freed by
> metadata_dst_free() in macsec_free_netdev(), while driver still use it
> for sending the packet.
> 
> [...]

Here is the summary with links:
  - [net,V2] macsec: Fix use-after-free while sending the offloading packet
    https://git.kernel.org/netdev/net/c/f1e54d11b210

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html