[v1] net/tls: refine the branch condition in tls_dev_event

[PATCH] net/tls: refine the branch condition in tls_dev_event

Posted by Kang Chen 2 years, 11 months ago

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

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)))
 			return NOTIFY_BAD;
 
 		if  (dev->tlsdev_ops &&
-- 
2.34.1

Re: [PATCH] net/tls: refine the branch condition in tls_dev_event

Posted by Horatiu Vultur 2 years, 11 months ago

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

[PATCH net v2] net/tls: refine the branch condition in tls_dev_event

Posted by Kang Chen 2 years, 11 months ago

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

Fixes: eeb2efaf36c7 ("net/tls: generalize the resync callback")
Signed-off-by: Kang Chen <void0red@gmail.com>
---
v2 -> v1: simplify the condition

 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..45b07162d062 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->tls_dev_resync))
 			return NOTIFY_BAD;
 
 		if  (dev->tlsdev_ops &&
-- 
2.34.1

Re: [PATCH net v2] net/tls: refine the branch condition in tls_dev_event

Posted by Jakub Kicinski 2 years, 11 months ago

On Fri, 17 Mar 2023 16:33:38 +0800 Kang Chen wrote:
> dev->tlsdev_ops may be null and cause null pointer dereference later.

It'd be a driver bug to report NETIF_F_HW_TLS_RX but not have ops.

Re: [PATCH net v2] net/tls: refine the branch condition in tls_dev_event

Posted by Horatiu Vultur 2 years, 11 months ago

The 03/17/2023 16:33, Kang Chen wrote:
> 
> dev->tlsdev_ops may be null and cause null pointer dereference later.

Reviewed-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> 
> Fixes: eeb2efaf36c7 ("net/tls: generalize the resync callback")
> Signed-off-by: Kang Chen <void0red@gmail.com>
> ---
> v2 -> v1: simplify the condition
> 
>  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..45b07162d062 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->tls_dev_resync))
>                         return NOTIFY_BAD;
> 
>                 if  (dev->tlsdev_ops &&
> --
> 2.34.1
> 

-- 
/Horatiu