[PATCH v1] net/mlx5: Fix null-ptr-deref in mlx5_create_inner_ttc_table()

[PATCH v1] net/mlx5: Fix null-ptr-deref in mlx5_create_inner_ttc_table()

[Henry Martin]

Add NULL check for mlx5_get_flow_namespace() returns in
mlx5_create_inner_ttc_table() to prevent NULL pointer dereference.

Fixes: 137f3d50ad2a ("net/mlx5: Support matching on l4_type for ttc_table")
Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/lib/fs_ttc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/fs_ttc.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/fs_ttc.c
index eb3bd9c7f66e..4e964ca5367e 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lib/fs_ttc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/fs_ttc.c
@@ -655,6 +655,8 @@ struct mlx5_ttc_table *mlx5_create_inner_ttc_table(struct mlx5_core_dev *dev,
 	}
 
 	ns = mlx5_get_flow_namespace(dev, params->ns_type);
+	if (!ns)
+		return ERR_PTR(-EOPNOTSUPP);
 	groups = use_l4_type ? &inner_ttc_groups[TTC_GROUPS_USE_L4_TYPE] :
 			       &inner_ttc_groups[TTC_GROUPS_DEFAULT];
 
-- 
2.34.1

Re: [PATCH] net/mlx5: Fix null-ptr-deref in mlx5_create_inner_ttc_table()

[Markus Elfring]

> Add NULL check for mlx5_get_flow_namespace() returns in
> mlx5_create_inner_ttc_table() to prevent NULL pointer dereference.

* You would like to adjust the error handling in some function implementations
  from a common subdirectory.
  How do you think about to offer such changes in a corresponding patch series?

* Can any other summary phrase variant become more desirable accordingly?

* Would any blank lines become also desirable after added statements?


Regards,
Markus

Re: [PATCH v1] net/mlx5: Fix null-ptr-deref in mlx5_create_inner_ttc_table()

[Paolo Abeni]

On 4/5/25 12:00 PM, Henry Martin wrote:
> Add NULL check for mlx5_get_flow_namespace() returns in
> mlx5_create_inner_ttc_table() to prevent NULL pointer dereference.
> 
> Fixes: 137f3d50ad2a ("net/mlx5: Support matching on l4_type for ttc_table")
> Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/lib/fs_ttc.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/fs_ttc.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/fs_ttc.c
> index eb3bd9c7f66e..4e964ca5367e 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/lib/fs_ttc.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/fs_ttc.c
> @@ -655,6 +655,8 @@ struct mlx5_ttc_table *mlx5_create_inner_ttc_table(struct mlx5_core_dev *dev,
>  	}
>  
>  	ns = mlx5_get_flow_namespace(dev, params->ns_type);
> +	if (!ns)
> +		return ERR_PTR(-EOPNOTSUPP);

I suspect the ns_type the caller always sets a valid 'ns_type', so the
NULL ptr is not really possible here.

At very least an empty line after the return statement will make the
code more readable and the commit message should be rewritten to better
describe the issue.

Similar considerations apply to the other mlx5 fixes.

Thanks,

Paolo

Re: [PATCH] net/mlx5: Fix null-ptr-deref in mlx5_create_inner_ttc_table()

[Markus Elfring]

…
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/fs_ttc.c
> > @@ -655,6 +655,8 @@ struct mlx5_ttc_table *mlx5_create_inner_ttc_table(struct mlx5_core_dev *dev,
> >  	}
> >
> >  	ns = mlx5_get_flow_namespace(dev, params->ns_type);
> > +	if (!ns)
> > +		return ERR_PTR(-EOPNOTSUPP);
>
> I suspect the ns_type the caller always sets a valid 'ns_type', so the
> NULL ptr is not really possible here.

Is there a need to mark such a check result as “unlikely”?

Regards,
Markus

Re: [PATCH] net/mlx5: Fix null-ptr-deref in mlx5_create_inner_ttc_table()

[Mark Bloch]

On 08/04/2025 15:25, Markus Elfring wrote:
> …
>>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/fs_ttc.c
>>> @@ -655,6 +655,8 @@ struct mlx5_ttc_table *mlx5_create_inner_ttc_table(struct mlx5_core_dev *dev,
>>>  	}
>>>
>>>  	ns = mlx5_get_flow_namespace(dev, params->ns_type);
>>> +	if (!ns)
>>> +		return ERR_PTR(-EOPNOTSUPP);
>>
>> I suspect the ns_type the caller always sets a valid 'ns_type', so the
>> NULL ptr is not really possible here.
> 
> Is there a need to mark such a check result as “unlikely”?
> 

Please don't. I'm fine with simply adding the check, as
Paolo suggested. When TTC was originally introduced, its
functionality was more limited, and reaching this point in the driver
meant we could be certain the namespace existed. Now that TTC has
become more advanced, adding this check makes sense and I'm okay with
it.

Mark

> Regards,
> Markus
> 

Re: [PATCH] net/mlx5: Fix null-ptr-deref in mlx5_create_inner_ttc_table()

[henry martin]

Thank you for the review. This check will be kept, and I'll follow Paolo's
suggestion about adding a blank line before the return statements in the v2.


Mark Bloch <mbloch@nvidia.com> 于2025年4月8日周二 23:01写道：
>
>
>
> On 08/04/2025 15:25, Markus Elfring wrote:
> > …
> >>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/fs_ttc.c
> >>> @@ -655,6 +655,8 @@ struct mlx5_ttc_table *mlx5_create_inner_ttc_table(struct mlx5_core_dev *dev,
> >>>     }
> >>>
> >>>     ns = mlx5_get_flow_namespace(dev, params->ns_type);
> >>> +   if (!ns)
> >>> +           return ERR_PTR(-EOPNOTSUPP);
> >>
> >> I suspect the ns_type the caller always sets a valid 'ns_type', so the
> >> NULL ptr is not really possible here.
> >
> > Is there a need to mark such a check result as “unlikely”?
> >
>
> Please don't. I'm fine with simply adding the check, as
> Paolo suggested. When TTC was originally introduced, its
> functionality was more limited, and reaching this point in the driver
> meant we could be certain the namespace existed. Now that TTC has
> become more advanced, adding this check makes sense and I'm okay with
> it.
>
> Mark
>
> > Regards,
> > Markus
> >
>

Re: net/mlx5: Fix null-ptr-deref in mlx5_create_inner_ttc_table()

[Markus Elfring]

> Thank you for the review. This check will be kept, and I'll follow Paolo's
> suggestion about adding a blank line before the return statements in the v2.

                                       after?

Regards,
Markus