Date:   Fri, 17 Mar 2023 09:15:13 +0100
From:   Horatiu Vultur <horatiu.vultur@microchip.com>
To:     Kang Chen <void0red@gmail.com>
CC:     <borisp@nvidia.com>, <john.fastabend@gmail.com>, <kuba@kernel.org>,
        <davem@davemloft.net>, <edumazet@google.com>, <pabeni@redhat.com>,
        <dirk.vandermerwe@netronome.com>, <netdev@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] net/tls: refine the branch condition in tls_dev_event
Message-ID: <20230317081513.ktllct3rqaisummm@soft-dev3-1>
References: <20230317071636.1028488-1-void0red@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
In-Reply-To: <20230317071636.1028488-1-void0red@gmail.com>
Precedence: bulk

The 03/17/2023 15:16, Kang Chen wrote:

Hi,

> 
> dev->tlsdev_ops may be null and cause null pointer dereference later.

In the subject of your patch, you should specify which tree is this
patch targeting. When you create the patch you can use:
git format-patch ... --subject-prefix "PATCH net" ...

> 
> Fixes: eeb2efaf36c7 ("net/tls: generalize the resync callback")
> Signed-off-by: Kang Chen <void0red@gmail.com>
> ---
>  net/tls/tls_device.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
> index a7cc4f9faac2..f30a8fe373c2 100644
> --- a/net/tls/tls_device.c
> +++ b/net/tls/tls_device.c
> @@ -1449,7 +1449,8 @@ static int tls_dev_event(struct notifier_block *this, unsigned long event,
>                 if (netif_is_bond_master(dev))
>                         return NOTIFY_DONE;
>                 if ((dev->features & NETIF_F_HW_TLS_RX) &&
> -                   !dev->tlsdev_ops->tls_dev_resync)
> +                  (!dev->tlsdev_ops || (dev->tlsdev_ops &&
> +                   !dev->tlsdev_ops->tls_dev_resync)))

This can be simply written like:
(!dev->tlvdev_ops || !dev->tlvdev_ops->tls_dev_resync)

On the second condition you know already that dev->tlvdev_ops is not
NULL.

>                         return NOTIFY_BAD;
> 
>                 if  (dev->tlsdev_ops &&
> --
> 2.34.1
> 

-- 
/Horatiu