linux-kernel - Re: Libertas: Association request to the driver failed

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]

Date:	Tue, 11 Aug 2009 14:24:26 -0400
From:	"John W. Linville" <linville@...driver.com>
To:	Roel Kluin <roel.kluin@...il.com>
Cc:	Daniel Mack <daniel@...aq.de>, libertas-dev@...ts.infradead.org,
	linux-wireless@...r.kernel.org, linux-kernel@...r.kernel.org,
	dcbw@...hat.com
Subject: Re: Libertas: Association request to the driver failed

Comments from the libertas crowd?  This seems a bit long for this
part of the cycle.

Should we just revert the original patch, then reapply it with this
one for 2.6.32?

John

On Tue, Aug 11, 2009 at 09:02:14AM +0200, Roel Kluin wrote:
> The size of the tmp buffer was too small, causing a regression
> 
> rates->rates has an arraysize of 1, so a memcpy with
> MAX_RATES (14) was already causing reads out of bounds.
> 
> In get_common_rates() the memset/memcpy can be moved upwards.
> 
> Signed-off-by: Roel Kluin <roel.kluin@...il.com>
> Tested-by: Daniel Mack <daniel@...aq.de>
> ---
> > Delta patch, please...
> 
> Here,
> 
> diff --git a/drivers/net/wireless/libertas/assoc.c b/drivers/net/wireless/libertas/assoc.c
> index d699737..ba0164a 100644
> --- a/drivers/net/wireless/libertas/assoc.c
> +++ b/drivers/net/wireless/libertas/assoc.c
> @@ -44,8 +44,8 @@ static int get_common_rates(struct lbs_private *priv,
>  	u16 *rates_size)
>  {
>  	u8 *card_rates = lbs_bg_rates;
> -	int ret = 0, i, j;
> -	u8 tmp[(ARRAY_SIZE(lbs_bg_rates) - 1) * (*rates_size - 1)];
> +	int i, j;
> +	u8 tmp[MAX_RATES * ARRAY_SIZE(lbs_bg_rates)];
>  	size_t tmp_size = 0;
>  
>  	/* For each rate in card_rates that exists in rate1, copy to tmp */
> @@ -62,20 +62,23 @@ static int get_common_rates(struct lbs_private *priv,
>  	lbs_deb_hex(LBS_DEB_JOIN, "common rates", tmp, tmp_size);
>  	lbs_deb_join("TX data rate 0x%02x\n", priv->cur_rate);
>  
> +	memset(rates, 0, *rates_size);
> +	*rates_size = min_t(u16, tmp_size, *rates_size);
> +	memcpy(rates, tmp, *rates_size);
> +
>  	if (!priv->enablehwauto) {
>  		for (i = 0; i < tmp_size; i++) {
>  			if (tmp[i] == priv->cur_rate)
> -				goto done;
> +				break;
> +		}
> +		if (i == tmp_size) {
> +			lbs_pr_alert("Previously set fixed data rate %#x isn't "
> +					"compatible with the network.\n",
> +					priv->cur_rate);
> +			return -1;
>  		}
> -		lbs_pr_alert("Previously set fixed data rate %#x isn't "
> -		       "compatible with the network.\n", priv->cur_rate);
> -		ret = -1;
>  	}
> -done:
> -	memset(rates, 0, *rates_size);
> -	*rates_size = min_t(int, tmp_size, *rates_size);
> -	memcpy(rates, tmp, *rates_size);
> -	return ret;
> +	return 0;
>  }
>  
>  
> @@ -319,8 +322,8 @@ static int lbs_associate(struct lbs_private *priv,
>  
>  	rates = (struct mrvl_ie_rates_param_set *) pos;
>  	rates->header.type = cpu_to_le16(TLV_TYPE_RATES);
> -	memcpy(&rates->rates, &bss->rates, MAX_RATES);
>  	tmplen = min_t(u16, ARRAY_SIZE(rates->rates), MAX_RATES);
> +	memcpy(&rates->rates, &bss->rates, tmplen);
>  	if (get_common_rates(priv, rates->rates, &tmplen)) {
>  		ret = -1;
>  		goto done;
> 

-- 
John W. Linville		Someday the world will need a hero, and you
linville@...driver.com			might be all we have.  Be ready.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/