[PATCH net-next] macsec: don't free NULL metadata_dst

[PATCH net-next] macsec: don't free NULL metadata_dst

[Sabrina Dubroca]

Commit 0a28bfd4971f added a metadata_dst to each tx_sc, but that's
only allocated when macsec_add_dev has run, which happens after device
registration. If the requested or computed SCI already exists, or if
linking to the lower device fails, we will panic because
metadata_dst_free can't handle NULL.

Reproducer:
    ip link add link $lower type macsec
    ip link add link $lower type macsec

Fixes: 0a28bfd4971f ("net/macsec: Add MACsec skb_metadata_dst Tx Data path support")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 drivers/net/macsec.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index 617f850bdb3a..ebcfe87bed23 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -3734,7 +3734,8 @@ static void macsec_free_netdev(struct net_device *dev)
 {
 	struct macsec_dev *macsec = macsec_priv(dev);
 
-	metadata_dst_free(macsec->secy.tx_sc.md_dst);
+	if (macsec->secy.tx_sc.md_dst)
+		metadata_dst_free(macsec->secy.tx_sc.md_dst);
 	free_percpu(macsec->stats);
 	free_percpu(macsec->secy.tx_sc.stats);
 
-- 
2.37.3

RE: [PATCH net-next] macsec: don't free NULL metadata_dst

[Raed Salem]

>From: Sabrina Dubroca <sd@queasysnail.net>
>Sent: Friday, 23 September 2022 12:07
>To: netdev@vger.kernel.org
>Cc: Lior Nahmanson <liorna@nvidia.com>; Raed Salem <raeds@nvidia.com>;
>Saeed Mahameed <saeedm@nvidia.com>; Sabrina Dubroca
><sd@queasysnail.net>
>Subject: [PATCH net-next] macsec: don't free NULL metadata_dst
>
>External email: Use caution opening links or attachments
>
>
>Commit 0a28bfd4971f added a metadata_dst to each tx_sc, but that's only
>allocated when macsec_add_dev has run, which happens after device
>registration. If the requested or computed SCI already exists, or if linking to
>the lower device fails, we will panic because metadata_dst_free can't handle
>NULL.
>
>Reproducer:
>    ip link add link $lower type macsec
>    ip link add link $lower type macsec
>
>Fixes: 0a28bfd4971f ("net/macsec: Add MACsec skb_metadata_dst Tx Data
>path support")
>Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
>---
> drivers/net/macsec.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c index
>617f850bdb3a..ebcfe87bed23 100644
>--- a/drivers/net/macsec.c
>+++ b/drivers/net/macsec.c
>@@ -3734,7 +3734,8 @@ static void macsec_free_netdev(struct net_device
>*dev)  {
>        struct macsec_dev *macsec = macsec_priv(dev);
>
>-       metadata_dst_free(macsec->secy.tx_sc.md_dst);
>+       if (macsec->secy.tx_sc.md_dst)
>+               metadata_dst_free(macsec->secy.tx_sc.md_dst);
>        free_percpu(macsec->stats);
>        free_percpu(macsec->secy.tx_sc.stats);
>
>--
>2.37.3
Acked by me

Re: [PATCH net-next] macsec: don't free NULL metadata_dst

[Jakub Kicinski]

On Sun, 25 Sep 2022 07:29:36 +0000 Raed Salem wrote:
> >Commit 0a28bfd4971f added a metadata_dst to each tx_sc, but that's only
> >allocated when macsec_add_dev has run, which happens after device
> >registration. If the requested or computed SCI already exists, or if linking to
> >the lower device fails, we will panic because metadata_dst_free can't handle
> >NULL.
> >
> >Reproducer:
> >    ip link add link $lower type macsec
> >    ip link add link $lower type macsec
> >
> >Fixes: 0a28bfd4971f ("net/macsec: Add MACsec skb_metadata_dst Tx Data
> >path support")
> >Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> 
> Acked by me

Thanks a lot for the review! Please prefer the full:

Acked-by: Raed Salem <raeds@nvidia.com>

format in the future, this way it will be picked up by the automation.

Re: [PATCH net-next] macsec: don't free NULL metadata_dst

[patchwork-bot+netdevbpf]

Hello:

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

On Fri, 23 Sep 2022 11:07:09 +0200 you wrote:
> Commit 0a28bfd4971f added a metadata_dst to each tx_sc, but that's
> only allocated when macsec_add_dev has run, which happens after device
> registration. If the requested or computed SCI already exists, or if
> linking to the lower device fails, we will panic because
> metadata_dst_free can't handle NULL.
> 
> Reproducer:
>     ip link add link $lower type macsec
>     ip link add link $lower type macsec
> 
> [...]

Here is the summary with links:
  - [net-next] macsec: don't free NULL metadata_dst
    https://git.kernel.org/netdev/net-next/c/c52add61c27e

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