[PATCH net-next] net/mlx5: HWS, Add an error check in hws_bwc_rule_complex_hash_node_get()

[PATCH net-next] net/mlx5: HWS, Add an error check in hws_bwc_rule_complex_hash_node_get()

[Dan Carpenter]

The rhashtable_lookup_get_insert_fast() function inserts an object into
the hashtable.  If the object was already present in the table it
returns a pointer to the original object.  If the object wasn't there
it returns NULL.  If there was an allocation error or some other kind
of failure, it returns an error pointer.

This caller needs to check for error pointers to avoid an error pointer
dereference.  Add the check.

Fixes: 17e0accac577 ("net/mlx5: HWS, support complex matchers")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
 .../net/ethernet/mellanox/mlx5/core/steering/hws/bwc_complex.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc_complex.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc_complex.c
index 5d30c5b094fc..6ae362fe2f36 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc_complex.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc_complex.c
@@ -1094,6 +1094,9 @@ hws_bwc_rule_complex_hash_node_get(struct mlx5hws_bwc_rule *bwc_rule,
 	old_node = rhashtable_lookup_get_insert_fast(refcount_hash,
 						     &node->hash_node,
 						     hws_refcount_hash);
+	if (IS_ERR(old_node))
+		return PTR_ERR(old_node);
+
 	if (old_node) {
 		/* Rule with the same tag already exists - update refcount */
 		refcount_inc(&old_node->refcount);
-- 
2.47.2

Re: [PATCH net-next] net/mlx5: HWS, Add an error check in hws_bwc_rule_complex_hash_node_get()

[Yevgeny Kliteynik]

On 28-May-25 11:12, Dan Carpenter wrote:
> The rhashtable_lookup_get_insert_fast() function inserts an object into
> the hashtable.  If the object was already present in the table it
> returns a pointer to the original object.  If the object wasn't there
> it returns NULL.  If there was an allocation error or some other kind
> of failure, it returns an error pointer.
> 
> This caller needs to check for error pointers to avoid an error pointer
> dereference.  Add the check.
> 
> Fixes: 17e0accac577 ("net/mlx5: HWS, support complex matchers")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
>   .../net/ethernet/mellanox/mlx5/core/steering/hws/bwc_complex.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc_complex.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc_complex.c
> index 5d30c5b094fc..6ae362fe2f36 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc_complex.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc_complex.c
> @@ -1094,6 +1094,9 @@ hws_bwc_rule_complex_hash_node_get(struct mlx5hws_bwc_rule *bwc_rule,
>   	old_node = rhashtable_lookup_get_insert_fast(refcount_hash,
>   						     &node->hash_node,
>   						     hws_refcount_hash);
> +	if (IS_ERR(old_node))
> +		return PTR_ERR(old_node);
> +

Agree with the need to check IS_ERR, but error flow is missing here.
Need to free the previously allocated IDA and node.

-- YK

>   	if (old_node) {
>   		/* Rule with the same tag already exists - update refcount */
>   		refcount_inc(&old_node->refcount);

Re: [PATCH net-next] net/mlx5: HWS, Add an error check in hws_bwc_rule_complex_hash_node_get()

[Dan Carpenter]

On Thu, May 29, 2025 at 01:26:17AM +0300, Yevgeny Kliteynik wrote:
> On 28-May-25 11:12, Dan Carpenter wrote:
> > The rhashtable_lookup_get_insert_fast() function inserts an object into
> > the hashtable.  If the object was already present in the table it
> > returns a pointer to the original object.  If the object wasn't there
> > it returns NULL.  If there was an allocation error or some other kind
> > of failure, it returns an error pointer.
> > 
> > This caller needs to check for error pointers to avoid an error pointer
> > dereference.  Add the check.
> > 
> > Fixes: 17e0accac577 ("net/mlx5: HWS, support complex matchers")
> > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> > ---
> >   .../net/ethernet/mellanox/mlx5/core/steering/hws/bwc_complex.c | 3 +++
> >   1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc_complex.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc_complex.c
> > index 5d30c5b094fc..6ae362fe2f36 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc_complex.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/bwc_complex.c
> > @@ -1094,6 +1094,9 @@ hws_bwc_rule_complex_hash_node_get(struct mlx5hws_bwc_rule *bwc_rule,
> >   	old_node = rhashtable_lookup_get_insert_fast(refcount_hash,
> >   						     &node->hash_node,
> >   						     hws_refcount_hash);
> > +	if (IS_ERR(old_node))
> > +		return PTR_ERR(old_node);
> > +
> 
> Agree with the need to check IS_ERR, but error flow is missing here.
> Need to free the previously allocated IDA and node.
> 

:/  Yeah.  Sorry...  I'll resend.

regards,
dan carpenter