Messages in this thread | | | Subject | Re: [PATCH 3/5] net/mlx4: fix some error handling in mlx4_multi_func_init() | From | Yishai Hadas <> | Date | Wed, 10 Feb 2016 11:40:07 +0200 |
| |
On 2/9/2016 10:11 PM, Rasmus Villemoes wrote: > The while loop after err_slaves should use post-decrement; otherwise > we'll fail to do the kfrees for i==0, and will run into out-of-bounds > accesses if the setup above failed already at i==0. > > The predecrement in the --port is ok, since ->vlan_filter is > (bizarrely) 1-indexed. But I'm changing 'if' to 'while' since it's a > bit ugly to rely on MLX4_MAX_PORTS being 2. > > [I'm not sure why one even bothers populating the ->vlan_filter array: > mlx4.h isn't #included by anything outside > drivers/net/ethernet/mellanox/mlx4/, and "git grep -C2 -w vlan_filter > drivers/net/ethernet/mellanox/mlx4/" seems to suggest that the > vlan_filter elements aren't used at all.] > > Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> > --- > drivers/net/ethernet/mellanox/mlx4/cmd.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/mellanox/mlx4/cmd.c b/drivers/net/ethernet/mellanox/mlx4/cmd.c > index d48d5793407d..bfe8234abbba 100644 > --- a/drivers/net/ethernet/mellanox/mlx4/cmd.c > +++ b/drivers/net/ethernet/mellanox/mlx4/cmd.c > @@ -2369,7 +2369,7 @@ int mlx4_multi_func_init(struct mlx4_dev *dev) > kzalloc(sizeof(struct mlx4_vlan_fltr), > GFP_KERNEL); > if (!s_state->vlan_filter[port]) { > - if (--port) > + while (--port)
Prefer to leave as-is. There is no way that mlx4 will ever have more than 2 ports.
> kfree(s_state->vlan_filter[port]); > goto err_slaves; > } > @@ -2429,7 +2429,7 @@ err_thread: > flush_workqueue(priv->mfunc.master.comm_wq); > destroy_workqueue(priv->mfunc.master.comm_wq); > err_slaves: > - while (--i) { > + while (i--) {
This fix is wrong as it hits the case that i arrived the last value then below code will access to a non valid entry in the array.
The expected fix should be: while (--i >= 0)
> for (port = 1; port <= MLX4_MAX_PORTS; port++) > kfree(priv->mfunc.master.slave_state[i].vlan_filter[port]); > } >
| |