[PATCH] net/mlx5e: fix potential null dereference in mlx5e_tc_nic_create_miss

netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH] net/mlx5e: fix potential null dereference in mlx5e_tc_nic_create_miss_table
@ 2025-04-02  9:32 Charles Han
  2025-04-03 18:28 ` Tariq Toukan
  0 siblings, 1 reply; 8+ messages in thread
From: Charles Han @ 2025-04-02  9:32 UTC (permalink / raw)
  To: saeedm, tariqt, leon, andrew+netdev, davem, edumazet, kuba,
	pabeni, maord, lariel, paulb
  Cc: netdev, linux-rdma, linux-kernel, Charles Han

mlx5_get_flow_namespace() may return a NULL pointer, dereferencing it
without NULL check may lead to NULL dereference.
Add a NULL check for ns.

Fixes: 66cb64e292d2 ("net/mlx5e: TC NIC mode, fix tc chains miss table")
Signed-off-by: Charles Han <hanchunchao@inspur.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index 9ba99609999f..9c524d8c0e5a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -5216,6 +5216,10 @@ static int mlx5e_tc_nic_create_miss_table(struct mlx5e_priv *priv)
 	ft_attr.level = MLX5E_TC_MISS_LEVEL;
 	ft_attr.prio = 0;
 	ns = mlx5_get_flow_namespace(priv->mdev, MLX5_FLOW_NAMESPACE_KERNEL);
+	if (!ns) {
+		mlx5_core_warn(priv->mdev, "Failed to get flow namespace\n");
+		return -EOPNOTSUPP;
+	}
 
 	*ft = mlx5_create_auto_grouped_flow_table(ns, &ft_attr);
 	if (IS_ERR(*ft)) {
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] net/mlx5e: fix potential null dereference in mlx5e_tc_nic_create_miss_table
  2025-04-02  9:32 [PATCH] net/mlx5e: fix potential null dereference in mlx5e_tc_nic_create_miss_table Charles Han
@ 2025-04-03 18:28 ` Tariq Toukan
  2025-04-07  7:20   ` [PATCH V2] " Charles Han
  0 siblings, 1 reply; 8+ messages in thread
From: Tariq Toukan @ 2025-04-03 18:28 UTC (permalink / raw)
  To: Charles Han, saeedm, leon, andrew+netdev, davem, edumazet, kuba,
	pabeni, maord, lariel, paulb
  Cc: netdev, linux-rdma, linux-kernel, Tariq Toukan



On 02/04/2025 12:32, Charles Han wrote:
> mlx5_get_flow_namespace() may return a NULL pointer, dereferencing it
> without NULL check may lead to NULL dereference.
> Add a NULL check for ns.
> 
> Fixes: 66cb64e292d2 ("net/mlx5e: TC NIC mode, fix tc chains miss table")
> Signed-off-by: Charles Han <hanchunchao@inspur.com>
> ---
>   drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> index 9ba99609999f..9c524d8c0e5a 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> @@ -5216,6 +5216,10 @@ static int mlx5e_tc_nic_create_miss_table(struct mlx5e_priv *priv)
>   	ft_attr.level = MLX5E_TC_MISS_LEVEL;
>   	ft_attr.prio = 0;
>   	ns = mlx5_get_flow_namespace(priv->mdev, MLX5_FLOW_NAMESPACE_KERNEL);
> +	if (!ns) {
> +		mlx5_core_warn(priv->mdev, "Failed to get flow namespace\n");

In this function netdev_err API is being used for error prints.

> +		return -EOPNOTSUPP;
> +	}
>   
>   	*ft = mlx5_create_auto_grouped_flow_table(ns, &ft_attr);
>   	if (IS_ERR(*ft)) {


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH V2] net/mlx5e: fix potential null dereference in mlx5e_tc_nic_create_miss_table
  2025-04-03 18:28 ` Tariq Toukan
@ 2025-04-07  7:20   ` Charles Han
  2025-04-07  9:29     ` Tariq Toukan
                       ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Charles Han @ 2025-04-07  7:20 UTC (permalink / raw)
  To: saeedm, tariqt, leon, andrew+netdev, davem, edumazet, kuba,
	pabeni, lariel, paulb, maord
  Cc: netdev, linux-rdma, linux-kernel, Charles Han

mlx5_get_flow_namespace() may return a NULL pointer, dereferencing it
without NULL check may lead to NULL dereference.
Add a NULL check for ns.

Fixes: 66cb64e292d2 ("net/mlx5e: TC NIC mode, fix tc chains miss table")
Signed-off-by: Charles Han <hanchunchao@inspur.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index 9ba99609999f..c2f23ac95c3d 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -5216,6 +5216,10 @@ static int mlx5e_tc_nic_create_miss_table(struct mlx5e_priv *priv)
 	ft_attr.level = MLX5E_TC_MISS_LEVEL;
 	ft_attr.prio = 0;
 	ns = mlx5_get_flow_namespace(priv->mdev, MLX5_FLOW_NAMESPACE_KERNEL);
+	if (!ns) {
+		netdev_err(priv->mdev, "Failed to get flow namespace\n");
+		return -EOPNOTSUPP;
+	}
 
 	*ft = mlx5_create_auto_grouped_flow_table(ns, &ft_attr);
 	if (IS_ERR(*ft)) {
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH V2] net/mlx5e: fix potential null dereference in mlx5e_tc_nic_create_miss_table
  2025-04-07  7:20   ` [PATCH V2] " Charles Han
@ 2025-04-07  9:29     ` Tariq Toukan
  2025-04-08  7:06       ` Charles Han
  2025-04-07 16:18     ` Simon Horman
  2025-04-09 14:16     ` Tariq Toukan
  2 siblings, 1 reply; 8+ messages in thread
From: Tariq Toukan @ 2025-04-07  9:29 UTC (permalink / raw)
  To: Charles Han, saeedm, tariqt, leon, andrew+netdev, davem, edumazet,
	kuba, pabeni, lariel, paulb, maord
  Cc: netdev, linux-rdma, linux-kernel



On 07/04/2025 10:20, Charles Han wrote:
> mlx5_get_flow_namespace() may return a NULL pointer, dereferencing it
> without NULL check may lead to NULL dereference.
> Add a NULL check for ns.
> 
> Fixes: 66cb64e292d2 ("net/mlx5e: TC NIC mode, fix tc chains miss table")
> Signed-off-by: Charles Han <hanchunchao@inspur.com>
> ---
>   drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> index 9ba99609999f..c2f23ac95c3d 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> @@ -5216,6 +5216,10 @@ static int mlx5e_tc_nic_create_miss_table(struct mlx5e_priv *priv)
>   	ft_attr.level = MLX5E_TC_MISS_LEVEL;
>   	ft_attr.prio = 0;
>   	ns = mlx5_get_flow_namespace(priv->mdev, MLX5_FLOW_NAMESPACE_KERNEL);
> +	if (!ns) {
> +		netdev_err(priv->mdev, "Failed to get flow namespace\n");
> +		return -EOPNOTSUPP;
> +	}
>   
>   	*ft = mlx5_create_auto_grouped_flow_table(ns, &ft_attr);
>   	if (IS_ERR(*ft)) {

Same question here, did it fail for you, or just saw it while reading 
the code?

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V2] net/mlx5e: fix potential null dereference in mlx5e_tc_nic_create_miss_table
  2025-04-07  9:29     ` Tariq Toukan
@ 2025-04-08  7:06       ` Charles Han
  2025-04-08 15:16         ` Mark Bloch
  0 siblings, 1 reply; 8+ messages in thread
From: Charles Han @ 2025-04-08  7:06 UTC (permalink / raw)
  To: Tariq Toukan
  Cc: saeedm, tariqt, leon, andrew+netdev, davem, edumazet, kuba,
	pabeni, lariel, paulb, maord, netdev, linux-rdma, linux-kernel

On Mon, Apr 07, 2025 at 12:29:22PM +0300, Tariq Toukan wrote:
> 
> 
> On 07/04/2025 10:20, Charles Han wrote:
> > mlx5_get_flow_namespace() may return a NULL pointer, dereferencing it
> > without NULL check may lead to NULL dereference.
> > Add a NULL check for ns.
> > 
> > Fixes: 66cb64e292d2 ("net/mlx5e: TC NIC mode, fix tc chains miss table")
> > Signed-off-by: Charles Han <hanchunchao@inspur.com>
> > ---
> >   drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 4 ++++
> >   1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> > index 9ba99609999f..c2f23ac95c3d 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> > @@ -5216,6 +5216,10 @@ static int mlx5e_tc_nic_create_miss_table(struct mlx5e_priv *priv)
> >   	ft_attr.level = MLX5E_TC_MISS_LEVEL;
> >   	ft_attr.prio = 0;
> >   	ns = mlx5_get_flow_namespace(priv->mdev, MLX5_FLOW_NAMESPACE_KERNEL);
> > +	if (!ns) {
> > +		netdev_err(priv->mdev, "Failed to get flow namespace\n");
> > +		return -EOPNOTSUPP;
> > +	}
> >   	*ft = mlx5_create_auto_grouped_flow_table(ns, &ft_attr);
> >   	if (IS_ERR(*ft)) {
> 
> Same question here, did it fail for you, or just saw it while reading the
> code?
I just saw it while reading the code.
I've been working on code vulnerability scanning recently.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V2] net/mlx5e: fix potential null dereference in mlx5e_tc_nic_create_miss_table
  2025-04-08  7:06       ` Charles Han
@ 2025-04-08 15:16         ` Mark Bloch
  0 siblings, 0 replies; 8+ messages in thread
From: Mark Bloch @ 2025-04-08 15:16 UTC (permalink / raw)
  To: Charles Han, Tariq Toukan
  Cc: saeedm, tariqt, leon, andrew+netdev, davem, edumazet, kuba,
	pabeni, lariel, paulb, maord, netdev, linux-rdma, linux-kernel



On 08/04/2025 10:06, Charles Han wrote:
> On Mon, Apr 07, 2025 at 12:29:22PM +0300, Tariq Toukan wrote:
>>
>>
>> On 07/04/2025 10:20, Charles Han wrote:
>>> mlx5_get_flow_namespace() may return a NULL pointer, dereferencing it
>>> without NULL check may lead to NULL dereference.
>>> Add a NULL check for ns.
>>>
>>> Fixes: 66cb64e292d2 ("net/mlx5e: TC NIC mode, fix tc chains miss table")
>>> Signed-off-by: Charles Han <hanchunchao@inspur.com>
>>> ---
>>>   drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 4 ++++
>>>   1 file changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
>>> index 9ba99609999f..c2f23ac95c3d 100644
>>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
>>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
>>> @@ -5216,6 +5216,10 @@ static int mlx5e_tc_nic_create_miss_table(struct mlx5e_priv *priv)
>>>   	ft_attr.level = MLX5E_TC_MISS_LEVEL;
>>>   	ft_attr.prio = 0;
>>>   	ns = mlx5_get_flow_namespace(priv->mdev, MLX5_FLOW_NAMESPACE_KERNEL);
>>> +	if (!ns) {
>>> +		netdev_err(priv->mdev, "Failed to get flow namespace\n");
>>> +		return -EOPNOTSUPP;
>>> +	}
>>>   	*ft = mlx5_create_auto_grouped_flow_table(ns, &ft_attr);
>>>   	if (IS_ERR(*ft)) {
>>
>> Same question here, did it fail for you, or just saw it while reading the
>> code?
> I just saw it while reading the code.
> I've been working on code vulnerability scanning recently.
> 

I don't believe this scenario can actually occur.
The function mlx5e_tc_nic_init() is called from mlx5e_init_nic_rx(),
and before that, we invoke mlx5e_create_flow_steering().

In mlx5e_create_flow_steering(), the first operation is:

<snip>
int mlx5e_create_flow_steering(struct mlx5e_flow_steering *fs,
                               struct mlx5e_rx_res *rx_res,
                               const struct mlx5e_profile *profile,
                               struct net_device *netdev)
{
        struct mlx5_flow_namespace *ns = mlx5_get_flow_namespace(fs->mdev,
                                                                 MLX5_FLOW_NAMESPACE_KERNEL);
        int err;

        if (!ns)
                return -EOPNOTSUPP;
</snip>

Note that MLX5_FLOW_NAMESPACE_KERNEL is allocated and initialized at
driver startup (as most/all namespaces), and it does not
change dynamically.

If mlx5e_create_flow_steering() fails, it indicates that
something fundamental isn't functioning correctly, and we
never proceed to the more advanced functionality (like tc).

Mark


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V2] net/mlx5e: fix potential null dereference in mlx5e_tc_nic_create_miss_table
  2025-04-07  7:20   ` [PATCH V2] " Charles Han
  2025-04-07  9:29     ` Tariq Toukan
@ 2025-04-07 16:18     ` Simon Horman
  2025-04-09 14:16     ` Tariq Toukan
  2 siblings, 0 replies; 8+ messages in thread
From: Simon Horman @ 2025-04-07 16:18 UTC (permalink / raw)
  To: Charles Han
  Cc: saeedm, tariqt, leon, andrew+netdev, davem, edumazet, kuba,
	pabeni, lariel, paulb, maord, netdev, linux-rdma, linux-kernel

On Mon, Apr 07, 2025 at 03:20:31PM +0800, Charles Han wrote:
> mlx5_get_flow_namespace() may return a NULL pointer, dereferencing it
> without NULL check may lead to NULL dereference.
> Add a NULL check for ns.
> 
> Fixes: 66cb64e292d2 ("net/mlx5e: TC NIC mode, fix tc chains miss table")
> Signed-off-by: Charles Han <hanchunchao@inspur.com>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> index 9ba99609999f..c2f23ac95c3d 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> @@ -5216,6 +5216,10 @@ static int mlx5e_tc_nic_create_miss_table(struct mlx5e_priv *priv)
>  	ft_attr.level = MLX5E_TC_MISS_LEVEL;
>  	ft_attr.prio = 0;
>  	ns = mlx5_get_flow_namespace(priv->mdev, MLX5_FLOW_NAMESPACE_KERNEL);
> +	if (!ns) {
> +		netdev_err(priv->mdev, "Failed to get flow namespace\n");

Hi Charles,

This does not seem to be correct. gcc-14.2.0 says:

drivers/net/ethernet/mellanox/mlx5/core/en_tc.c: In function 'mlx5e_tc_nic_create_miss_table':
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c:5220:32: error: passing argument 1 of 'netdev_err' from incompatible pointer type [-Wincompatible-pointer-types]
 5220 |                 netdev_err(priv->mdev, "Failed to get flow namespace\n");
      |                            ~~~~^~~~~~
      |                                |
      |                                struct mlx5_core_dev *
In file included from ./include/linux/skbuff.h:39,
                 from ./include/linux/netlink.h:7,
                 from ./include/net/flow_offload.h:6,
                 from drivers/net/ethernet/mellanox/mlx5/core/en_tc.c:34:
./include/net/net_debug.h:20:42: note: expected 'const struct net_device *' but argument is of type 'struct mlx5_core_dev *'
   20 | void netdev_err(const struct net_device *dev, const char *format, ...);
      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~^~~

...

-- 
pw-bot: changes-requested

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V2] net/mlx5e: fix potential null dereference in mlx5e_tc_nic_create_miss_table
  2025-04-07  7:20   ` [PATCH V2] " Charles Han
  2025-04-07  9:29     ` Tariq Toukan
  2025-04-07 16:18     ` Simon Horman
@ 2025-04-09 14:16     ` Tariq Toukan
  2 siblings, 0 replies; 8+ messages in thread
From: Tariq Toukan @ 2025-04-09 14:16 UTC (permalink / raw)
  To: Charles Han, saeedm, leon, andrew+netdev, davem, edumazet, kuba,
	pabeni, lariel, paulb, maord, Henry Martin
  Cc: netdev, linux-rdma, linux-kernel, Tariq Toukan



On 07/04/2025 10:20, Charles Han wrote:
> mlx5_get_flow_namespace() may return a NULL pointer, dereferencing it
> without NULL check may lead to NULL dereference.
> Add a NULL check for ns.
> 
> Fixes: 66cb64e292d2 ("net/mlx5e: TC NIC mode, fix tc chains miss table")
> Signed-off-by: Charles Han <hanchunchao@inspur.com>
> ---
>   drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> index 9ba99609999f..c2f23ac95c3d 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> @@ -5216,6 +5216,10 @@ static int mlx5e_tc_nic_create_miss_table(struct mlx5e_priv *priv)
>   	ft_attr.level = MLX5E_TC_MISS_LEVEL;
>   	ft_attr.prio = 0;
>   	ns = mlx5_get_flow_namespace(priv->mdev, MLX5_FLOW_NAMESPACE_KERNEL);
> +	if (!ns) {
> +		netdev_err(priv->mdev, "Failed to get flow namespace\n");
> +		return -EOPNOTSUPP;
> +	}
>   
>   	*ft = mlx5_create_auto_grouped_flow_table(ns, &ft_attr);
>   	if (IS_ERR(*ft)) {


Too many similar patches submitted individually in parallel by multiple 
authors..

One can easily lose track.

Please gather similar patches in a series, provide cover letter and 
target branch.

Tariq.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2025-04-09 14:16 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-02  9:32 [PATCH] net/mlx5e: fix potential null dereference in mlx5e_tc_nic_create_miss_table Charles Han
2025-04-03 18:28 ` Tariq Toukan
2025-04-07  7:20   ` [PATCH V2] " Charles Han
2025-04-07  9:29     ` Tariq Toukan
2025-04-08  7:06       ` Charles Han
2025-04-08 15:16         ` Mark Bloch
2025-04-07 16:18     ` Simon Horman
2025-04-09 14:16     ` Tariq Toukan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).