netdev - Re: [PATCH net-next v3 2/3] nfp: add framework to support ipsec offloading

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 PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 6 Nov 2022 21:39:51 +0200
From:   Leon Romanovsky <leon@...nel.org>
To:     Simon Horman <simon.horman@...igine.com>
Cc:     David Miller <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Steffen Klassert <steffen.klassert@...unet.com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        Chentian Liu <chengtian.liu@...igine.com>,
        Huanhuan Wang <huanhuan.wang@...igine.com>,
        Yinjun Zhang <yinjun.zhang@...igine.com>,
        Louis Peens <louis.peens@...igine.com>, netdev@...r.kernel.org,
        oss-drivers@...igine.com
Subject: Re: [PATCH net-next v3 2/3] nfp: add framework to support ipsec
 offloading

On Tue, Nov 01, 2022 at 12:02:47PM +0100, Simon Horman wrote:
> From: Huanhuan Wang <huanhuan.wang@...igine.com>
> 
> A new metadata type and config structure are introduced to
> interact with firmware to support ipsec offloading. This
> feature relies on specific firmware that supports ipsec
> encrypt/decrypt by advertising related capability bit.
> 
> The xfrm callbacks which interact with upper layer are
> implemented in the following patch.
> 
> Based on initial work of Norm Bagley <norman.bagley@...ronome.com>.
> 
> Signed-off-by: Huanhuan Wang <huanhuan.wang@...igine.com>
> Reviewed-by: Louis Peens <louis.peens@...igine.com>
> Signed-off-by: Simon Horman <simon.horman@...igine.com>
> ---
>  drivers/net/ethernet/netronome/Kconfig        |  11 ++
>  drivers/net/ethernet/netronome/nfp/Makefile   |   2 +
>  .../ethernet/netronome/nfp/crypto/crypto.h    |  23 ++++
>  .../net/ethernet/netronome/nfp/crypto/ipsec.c | 105 ++++++++++++++++++
>  drivers/net/ethernet/netronome/nfp/nfd3/dp.c  |  58 ++++++++--
>  .../net/ethernet/netronome/nfp/nfd3/ipsec.c   |  18 +++
>  .../net/ethernet/netronome/nfp/nfd3/nfd3.h    |   8 ++
>  drivers/net/ethernet/netronome/nfp/nfp_net.h  |   9 ++
>  .../ethernet/netronome/nfp/nfp_net_common.c   |   3 +
>  .../net/ethernet/netronome/nfp/nfp_net_ctrl.h |   4 +
>  10 files changed, 231 insertions(+), 10 deletions(-)
>  create mode 100644 drivers/net/ethernet/netronome/nfp/crypto/ipsec.c
>  create mode 100644 drivers/net/ethernet/netronome/nfp/nfd3/ipsec.c

<...>

> +void nfp_net_ipsec_clean(struct nfp_net *nn)
> +{
> +	if (!(nn->cap_w1 & NFP_NET_CFG_CTRL_IPSEC))
> +		return;
> +	xa_destroy(&nn->xa_ipsec);

You shouldn't use xa_destroy() here as if you have entries in xa_ipsec,
you won't release them and leak memory without any warning. Most likely,
the WARN_ON(!xa_empty(&nn->xa_ipsec)) is what you want here.

The rest code is ok.

Thanks,
Reviewed-by: Leon Romanovsky <leonro@...dia.com>