| Message ID | 60f2a1965fe553e2cade9472407d0fafff8de8ce.1663923580.git.sd@queasysnail.net (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | c52add61c27ea23501be82a34854edd98e10e061 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] macsec: don't free NULL metadata_dst | expand |
>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
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.
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!
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);
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(-)