Message ID | 1474212029-1052-2-git-send-email-ogerlitz@mellanox.com |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
On Sun, Sep 18, 2016 at 06:20:27PM +0300, Or Gerlitz wrote: > From: Roi Dayan <roid@mellanox.com> > > 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 <roid@mellanox.com> > Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com> > --- > 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 >
On Sun, Sep 18, 2016 at 9:02 PM, Leon Romanovsky <leon@kernel.org> wrote: > On Sun, Sep 18, 2016 at 06:20:27PM +0300, Or Gerlitz wrote: >> From: Roi Dayan <roid@mellanox.com> >> @@ -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.
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;