Re: [PATCH net 1/3] net/mlx5: Fix flow counter bulk command out mailbox allocation
On Sun, Sep 18, 2016 at 9:02 PM, Leon Romanovsky wrote: > On Sun, Sep 18, 2016 at 06:20:27PM +0300, Or Gerlitz wrote: >> From: Roi Dayan >> @@ -425,11 +425,11 @@ struct mlx5_cmd_fc_bulk * >> mlx5_cmd_fc_bulk_alloc(struct mlx5_core_dev *dev, u16 id, int num) >> { >> struct mlx5_cmd_fc_bulk *b; >> - int outlen = sizeof(*b) + >> + int outlen = >> MLX5_ST_SZ_BYTES(query_flow_counter_out) + >> MLX5_ST_SZ_BYTES(traffic_counter) * num; >> >> - b = kzalloc(outlen, GFP_KERNEL); >> + b = kzalloc(sizeof(*b) + outlen, GFP_KERNEL); >> if (!b) >> return NULL; > ^ very controversial decision. > The code flow mlx5_fc_stats_query->mlx5_cmd_fc_bulk_alloc->kzalloc > failure is the same for success scenario too. Sure, we will look on your comment and if needed come up with a cleanup patch for net-next (4.9) > It is not related to the proposed patch. Correct, the proposed patch fixes a memory corruption that we want to sort out for net (4.8) Or.
Re: [PATCH net 1/3] net/mlx5: Fix flow counter bulk command out mailbox allocation
On Sun, Sep 18, 2016 at 06:20:27PM +0300, Or Gerlitz wrote: > From: Roi Dayan > > The FW command output length should be only the length of struct > mlx5_cmd_fc_bulk out field. Failing to do so will cause the memcpy > call which is invoked later in the driver to write over wrong memory > address and corrupt kernel memory which results in random crashes. > > This bug was found using the kernel address sanitizer (kasan). > > Fixes: a351a1b03bf1 ('net/mlx5: Introduce bulk reading of flow counters') > Signed-off-by: Roi Dayan > Signed-off-by: Or Gerlitz > --- > drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c > b/drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c > index 9134010..287ade1 100644 > --- a/drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c > +++ b/drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c > @@ -425,11 +425,11 @@ struct mlx5_cmd_fc_bulk * > mlx5_cmd_fc_bulk_alloc(struct mlx5_core_dev *dev, u16 id, int num) > { > struct mlx5_cmd_fc_bulk *b; > - int outlen = sizeof(*b) + > + int outlen = > MLX5_ST_SZ_BYTES(query_flow_counter_out) + > MLX5_ST_SZ_BYTES(traffic_counter) * num; > > - b = kzalloc(outlen, GFP_KERNEL); > + b = kzalloc(sizeof(*b) + outlen, GFP_KERNEL); > if (!b) > return NULL; ^ very controversial decision. The code flow mlx5_fc_stats_query->mlx5_cmd_fc_bulk_alloc->kzalloc failure is the same for success scenario too. It is not related to the proposed patch. > > -- > 2.3.7 > signature.asc Description: PGP signature