linux-kernel - Re: [PATCH 3/5] net/mlx4: fix some error handling in mlx4_multi_func

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <56BCB082.4030701@redhat.com>
Date:	Thu, 11 Feb 2016 11:02:10 -0500
From:	Doug Ledford <dledford@...hat.com>
To:	Rasmus Villemoes <linux@...musvillemoes.dk>,
	Yishai Hadas <yishaih@...lanox.com>
Cc:	netdev@...r.kernel.org, linux-rdma@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/5] net/mlx4: fix some error handling in
 mlx4_multi_func_init()

On 02/09/2016 03: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@...musvillemoes.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)
>  						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--) {
>  		for (port = 1; port <= MLX4_MAX_PORTS; port++)
>  			kfree(priv->mfunc.master.slave_state[i].vlan_filter[port]);
>  	}
> 

I'm modifying your patch slightly (dropping the first hunk, it isn't
really necessary as Yishai pointed out in review) and adjusting the
description to compensate.  I'll apply the result to my next for-rc series.

-- 
Doug Ledford <dledford@...hat.com>
              GPG KeyID: 0E572FDD



Download attachment "signature.asc" of type "application/pgp-signature" (885 bytes)