| Message ID | 20200603175436.GD18931@mwanda (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | net/mlx5: E-Switch, Fix some error pointer dereferences | expand |
+ netdev On Wed, Jun 03, 2020 at 08:54:36PM +0300, Dan Carpenter wrote: > We can't leave "counter" set to an error pointer. Otherwise either it > will lead to an error pointer dereference later in the function or it > leads to an error pointer dereference when we call mlx5_fc_destroy(). > > Fixes: 07bab9502641d ("net/mlx5: E-Switch, Refactor eswitch ingress acl codes") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > .../net/ethernet/mellanox/mlx5/core/esw/acl/ingress_lgcy.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/acl/ingress_lgcy.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/acl/ingress_lgcy.c > index 9bda4fe2eafa7..5dc335e621c57 100644 > --- a/drivers/net/ethernet/mellanox/mlx5/core/esw/acl/ingress_lgcy.c > +++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/acl/ingress_lgcy.c > @@ -162,10 +162,12 @@ int esw_acl_ingress_lgcy_setup(struct mlx5_eswitch *esw, > > if (MLX5_CAP_ESW_INGRESS_ACL(esw->dev, flow_counter)) { > counter = mlx5_fc_create(esw->dev, false); > - if (IS_ERR(counter)) > + if (IS_ERR(counter)) { > esw_warn(esw->dev, > "vport[%d] configure ingress drop rule counter failed\n", > vport->vport); > + counter = NULL; > + } > vport->ingress.legacy.drop_counter = counter; > } > > @@ -272,7 +274,7 @@ void esw_acl_ingress_lgcy_cleanup(struct mlx5_eswitch *esw, > esw_acl_ingress_table_destroy(vport); > > clean_drop_counter: > - if (!IS_ERR_OR_NULL(vport->ingress.legacy.drop_counter)) { > + if (vport->ingress.legacy.drop_counter) { > mlx5_fc_destroy(esw->dev, vport->ingress.legacy.drop_counter); > vport->ingress.legacy.drop_counter = NULL; > } > -- > 2.26.2 >
On Thu, Jun 04, 2020 at 01:32:55PM +0300, Leon Romanovsky wrote: > + netdev > This is sort of useless. What's netdev going to do with a patch they can't apply? I assumed that mellanox was going to take this through their tree... Should I resend the other mlx5 patch as well? regards, dan carpenter
On Fri, Jun 05, 2020 at 01:52:03PM +0300, Dan Carpenter wrote: > On Thu, Jun 04, 2020 at 01:32:55PM +0300, Leon Romanovsky wrote: > > + netdev > > > > This is sort of useless. What's netdev going to do with a patch they > can't apply? I assumed that mellanox was going to take this through > their tree... Right, but it will be picked by Saeed who will send it to netdev later as PR. CCing netdev saves extra review at that stage. > > Should I resend the other mlx5 patch as well? I don't think so. > > regards, > dan carpenter >
On Sun, Jun 07, 2020 at 09:25:55AM +0300, Leon Romanovsky wrote: > On Fri, Jun 05, 2020 at 01:52:03PM +0300, Dan Carpenter wrote: > > On Thu, Jun 04, 2020 at 01:32:55PM +0300, Leon Romanovsky wrote: > > > + netdev > > > > > > > This is sort of useless. What's netdev going to do with a patch they > > can't apply? I assumed that mellanox was going to take this through > > their tree... > > Right, but it will be picked by Saeed who will send it to netdev later > as PR. CCing netdev saves extra review at that stage. Okay. I will try to remember this in the future. I'll try put [PATCH mlx5-next] in the subject even when it applies to the net tree. regards, dan carpenter
On Mon, 2020-06-08 at 16:31 +0300, Dan Carpenter wrote: > On Sun, Jun 07, 2020 at 09:25:55AM +0300, Leon Romanovsky wrote: > > On Fri, Jun 05, 2020 at 01:52:03PM +0300, Dan Carpenter wrote: > > > On Thu, Jun 04, 2020 at 01:32:55PM +0300, Leon Romanovsky wrote: > > > > + netdev > > > > > > > > > > This is sort of useless. What's netdev going to do with a patch > > > they > > > can't apply? I assumed that mellanox was going to take this > > > through > > > their tree... > > > > Right, but it will be picked by Saeed who will send it to netdev > > later > > as PR. CCing netdev saves extra review at that stage. > > Okay. I will try to remember this in the future. I'll try put > [PATCH mlx5-next] in the subject even when it applies to the net > tree. Thanks Dan for the patch. netdev is always open for fixes. Applied to net-mlx5, will send this in today PR to net. -Saeed. > > regards, > dan carpenter >
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/acl/ingress_lgcy.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/acl/ingress_lgcy.c index 9bda4fe2eafa7..5dc335e621c57 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/esw/acl/ingress_lgcy.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/acl/ingress_lgcy.c @@ -162,10 +162,12 @@ int esw_acl_ingress_lgcy_setup(struct mlx5_eswitch *esw, if (MLX5_CAP_ESW_INGRESS_ACL(esw->dev, flow_counter)) { counter = mlx5_fc_create(esw->dev, false); - if (IS_ERR(counter)) + if (IS_ERR(counter)) { esw_warn(esw->dev, "vport[%d] configure ingress drop rule counter failed\n", vport->vport); + counter = NULL; + } vport->ingress.legacy.drop_counter = counter; } @@ -272,7 +274,7 @@ void esw_acl_ingress_lgcy_cleanup(struct mlx5_eswitch *esw, esw_acl_ingress_table_destroy(vport); clean_drop_counter: - if (!IS_ERR_OR_NULL(vport->ingress.legacy.drop_counter)) { + if (vport->ingress.legacy.drop_counter) { mlx5_fc_destroy(esw->dev, vport->ingress.legacy.drop_counter); vport->ingress.legacy.drop_counter = NULL; }
We can't leave "counter" set to an error pointer. Otherwise either it will lead to an error pointer dereference later in the function or it leads to an error pointer dereference when we call mlx5_fc_destroy(). Fixes: 07bab9502641d ("net/mlx5: E-Switch, Refactor eswitch ingress acl codes") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- .../net/ethernet/mellanox/mlx5/core/esw/acl/ingress_lgcy.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)