| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <130ac9b1-9ac5-4a4a-2fa3-c1681f639940@intel.com>
Date: Mon, 28 Nov 2022 15:36:13 -0800
From: Jacob Keller <jacob.e.keller@...el.com>
To: Saeed Mahameed <saeed@...nel.org>,
"David S. Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Eric Dumazet <edumazet@...gle.com>
CC: Saeed Mahameed <saeedm@...dia.com>, <netdev@...r.kernel.org>,
Tariq Toukan <tariqt@...dia.com>,
Raed Salem <raeds@...dia.com>, Emeel Hakim <ehakim@...dia.com>
Subject: Re: [net 09/15] net/mlx5e: MACsec, fix memory leak when MACsec device
is deleted
On 11/24/2022 12:10 AM, Saeed Mahameed wrote:
> From: Raed Salem <raeds@...dia.com>
>
> When the MACsec netdevice is deleted, all related Rx/Tx HW/SW
> states should be released/deallocated, however currently part
> of the Rx security channel association data is not cleaned
> properly, hence the memory leaks.
>
> Fix by make sure all related Rx Sc resources are cleaned/freed,
> while at it improve code by grouping release SC context in a
> function so it can be used in both delete MACsec device and
> delete Rx SC operations.
>
> Fixes: 5a39816a75e5 ("net/mlx5e: Add MACsec offload SecY support")
> Signed-off-by: Raed Salem <raeds@...dia.com>
> Reviewed-by: Emeel Hakim <ehakim@...dia.com>
> Signed-off-by: Saeed Mahameed <saeedm@...dia.com>
> ---
> .../mellanox/mlx5/core/en_accel/macsec.c | 77 ++++++++-----------
> 1 file changed, 33 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c
> index 96fa553ef93a..b51de07d5bad 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/macsec.c
> @@ -823,16 +823,43 @@ static int mlx5e_macsec_upd_rxsc(struct macsec_context *ctx)
> return err;
> }
>
Reviewed-by: Jacob Keller <jacob.e.keller@...el.com>
> +static void macsec_del_rxsc_ctx(struct mlx5e_macsec *macsec, struct mlx5e_macsec_rx_sc *rx_sc)
> +{
> + struct mlx5e_macsec_sa *rx_sa;
> + int i;
> +
> + for (i = 0; i < MACSEC_NUM_AN; ++i) {
> + rx_sa = rx_sc->rx_sa[i];
> + if (!rx_sa)
> + continue;
> +
> + mlx5e_macsec_cleanup_sa(macsec, rx_sa, false);
> + mlx5_destroy_encryption_key(macsec->mdev, rx_sa->enc_key_id);
> +
> + kfree(rx_sa);
> + rx_sc->rx_sa[i] = NULL;
> + }
> +
> + /* At this point the relevant MACsec offload Rx rule already removed at
> + * mlx5e_macsec_cleanup_sa need to wait for datapath to finish current
> + * Rx related data propagating using xa_erase which uses rcu to sync,
> + * once fs_id is erased then this rx_sc is hidden from datapath.
> + */
> + list_del_rcu(&rx_sc->rx_sc_list_element);
> + xa_erase(&macsec->sc_xarray, rx_sc->sc_xarray_element->fs_id);
> + metadata_dst_free(rx_sc->md_dst);
> + kfree(rx_sc->sc_xarray_element);
> + kfree_rcu(rx_sc);
> +}
> +
> static int mlx5e_macsec_del_rxsc(struct macsec_context *ctx)
> {
> struct mlx5e_priv *priv = netdev_priv(ctx->netdev);
> struct mlx5e_macsec_device *macsec_device;
> struct mlx5e_macsec_rx_sc *rx_sc;
> - struct mlx5e_macsec_sa *rx_sa;
> struct mlx5e_macsec *macsec;
> struct list_head *list;
> int err = 0;
> - int i;
>
> mutex_lock(&priv->macsec->lock);
>
> @@ -854,31 +881,7 @@ static int mlx5e_macsec_del_rxsc(struct macsec_context *ctx)
> goto out;
> }
>
> - for (i = 0; i < MACSEC_NUM_AN; ++i) {
> - rx_sa = rx_sc->rx_sa[i];
> - if (!rx_sa)
> - continue;
> -
> - mlx5e_macsec_cleanup_sa(macsec, rx_sa, false);
> - mlx5_destroy_encryption_key(macsec->mdev, rx_sa->enc_key_id);
> -
> - kfree(rx_sa);
> - rx_sc->rx_sa[i] = NULL;
> - }
> -
> -/*
> - * At this point the relevant MACsec offload Rx rule already removed at
> - * mlx5e_macsec_cleanup_sa need to wait for datapath to finish current
> - * Rx related data propagating using xa_erase which uses rcu to sync,
> - * once fs_id is erased then this rx_sc is hidden from datapath.
> - */
> - list_del_rcu(&rx_sc->rx_sc_list_element);
> - xa_erase(&macsec->sc_xarray, rx_sc->sc_xarray_element->fs_id);
> - metadata_dst_free(rx_sc->md_dst);
> - kfree(rx_sc->sc_xarray_element);
> -
> - kfree_rcu(rx_sc);
> -
> + macsec_del_rxsc_ctx(macsec, rx_sc);
> out:
> mutex_unlock(&macsec->lock);
>
> @@ -1239,7 +1242,6 @@ static int mlx5e_macsec_del_secy(struct macsec_context *ctx)
> struct mlx5e_priv *priv = netdev_priv(ctx->netdev);
> struct mlx5e_macsec_device *macsec_device;
> struct mlx5e_macsec_rx_sc *rx_sc, *tmp;
> - struct mlx5e_macsec_sa *rx_sa;
> struct mlx5e_macsec_sa *tx_sa;
> struct mlx5e_macsec *macsec;
> struct list_head *list;
> @@ -1268,28 +1270,15 @@ static int mlx5e_macsec_del_secy(struct macsec_context *ctx)
> }
>
> list = &macsec_device->macsec_rx_sc_list_head;
> - list_for_each_entry_safe(rx_sc, tmp, list, rx_sc_list_element) {
> - for (i = 0; i < MACSEC_NUM_AN; ++i) {
> - rx_sa = rx_sc->rx_sa[i];
> - if (!rx_sa)
> - continue;
> -
> - mlx5e_macsec_cleanup_sa(macsec, rx_sa, false);
> - mlx5_destroy_encryption_key(macsec->mdev, rx_sa->enc_key_id);
> - kfree(rx_sa);
> - rx_sc->rx_sa[i] = NULL;
> - }
> -
> - list_del_rcu(&rx_sc->rx_sc_list_element);
> -
> - kfree_rcu(rx_sc);
> - }
> + list_for_each_entry_safe(rx_sc, tmp, list, rx_sc_list_element)
> + macsec_del_rxsc_ctx(macsec, rx_sc);
>
> kfree(macsec_device->dev_addr);
> macsec_device->dev_addr = NULL;
>
> list_del_rcu(&macsec_device->macsec_device_list_element);
> --macsec->num_of_devices;
> + kfree(macsec_device);
>
> out:
> mutex_unlock(&macsec->lock);
Powered by blists - more mailing lists