2023-03-17 07:16:48

by void0red

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

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 <[email protected]>
---
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

2023-03-17 08:17:01

by Horatiu Vultur

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

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 <[email protected]>
> ---
> 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

2023-03-17 08:35:25

by void0red

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

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 <[email protected]>
---
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

2023-03-17 09:57:08

by Horatiu Vultur

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

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 <[email protected]>
>
> Fixes: eeb2efaf36c7 ("net/tls: generalize the resync callback")
> Signed-off-by: Kang Chen <[email protected]>
> ---
> 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

2023-03-17 19:54:51

by Jakub Kicinski

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

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.