| Message ID | 20241021100309.234125-1-tariqt@nvidia.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | f1e54d11b210b53d418ff1476c6b58a2f434dfc0 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,V2] macsec: Fix use-after-free while sending the offloading packet | expand |
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/ ...
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>
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!
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);