netdev - Re: [PATCH iwl-net] ixgbe: avoid sleeping allocation in ixgbe_ipsec_vf_add

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

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

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

Date: Tue, 5 Mar 2024 09:16:27 -0800
From: "Nelson, Shannon" <shannon.nelson@....com>
To: Przemek Kitszel <przemyslaw.kitszel@...el.com>,
 intel-wired-lan@...ts.osuosl.org
Cc: netdev@...r.kernel.org, Tony Nguyen <anthony.l.nguyen@...el.com>,
 lukasz.czapnik@...el.com, Dan Carpenter <dan.carpenter@...aro.org>,
 Michal Kubiak <michal.kubiak@...el.com>
Subject: Re: [PATCH iwl-net] ixgbe: avoid sleeping allocation in
 ixgbe_ipsec_vf_add_sa()

On 3/5/2024 8:02 AM, Przemek Kitszel wrote:
> Change kzalloc() flags used in ixgbe_ipsec_vf_add_sa() to GFP_ATOMIC, to
> avoid sleeping in IRQ context.
> 
> Dan Carpenter, with the help of Smatch, has found following issue:
> The patch eda0333ac293: "ixgbe: add VF IPsec management" from Aug 13,
> 2018 (linux-next), leads to the following Smatch static checker
> warning: drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c:917 ixgbe_ipsec_vf_add_sa()
>          warn: sleeping in IRQ context
> 
> The call tree that Smatch is worried about is:
> ixgbe_msix_other() <- IRQ handler
> -> ixgbe_msg_task()
>     -> ixgbe_rcv_msg_from_vf()
>        -> ixgbe_ipsec_vf_add_sa()
> 
> Reported-by: Dan Carpenter <dan.carpenter@...aro.org>
> Link: https://lore.kernel.org/intel-wired-lan/db31a0b0-4d9f-4e6b-aed8-88266eb5665c@moroto.mountain
> Reviewed-by: Michal Kubiak <michal.kubiak@...el.com>
> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>

Thanks, that should work.

Reviewed-by: Shannon Nelson <shannon.nelson@....com>


> ---
>   drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
> index 13a6fca31004..866024f2b9ee 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
> @@ -914,7 +914,13 @@ int ixgbe_ipsec_vf_add_sa(struct ixgbe_adapter *adapter, u32 *msgbuf, u32 vf)
>                  goto err_out;
>          }
> 
> -       xs = kzalloc(sizeof(*xs), GFP_KERNEL);
> +       algo = xfrm_aead_get_byname(aes_gcm_name, IXGBE_IPSEC_AUTH_BITS, 1);
> +       if (unlikely(!algo)) {
> +               err = -ENOENT;
> +               goto err_out;
> +       }
> +
> +       xs = kzalloc(sizeof(*xs), GFP_ATOMIC);
>          if (unlikely(!xs)) {
>                  err = -ENOMEM;
>                  goto err_out;
> @@ -930,14 +936,8 @@ int ixgbe_ipsec_vf_add_sa(struct ixgbe_adapter *adapter, u32 *msgbuf, u32 vf)
>                  memcpy(&xs->id.daddr.a4, sam->addr, sizeof(xs->id.daddr.a4));
>          xs->xso.dev = adapter->netdev;
> 
> -       algo = xfrm_aead_get_byname(aes_gcm_name, IXGBE_IPSEC_AUTH_BITS, 1);
> -       if (unlikely(!algo)) {
> -               err = -ENOENT;
> -               goto err_xs;
> -       }
> -
>          aead_len = sizeof(*xs->aead) + IXGBE_IPSEC_KEY_BITS / 8;
> -       xs->aead = kzalloc(aead_len, GFP_KERNEL);
> +       xs->aead = kzalloc(aead_len, GFP_ATOMIC);
>          if (unlikely(!xs->aead)) {
>                  err = -ENOMEM;
>                  goto err_xs;
> 
> base-commit: 9b23fceb4158a3636ce4a2bda28ab03dcfa6a26f
> --
> 2.43.0
>