On Tue, 2009-08-11 at 14:24 -0400, John W. Linville wrote:
> 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?
I'd feel more comfortable with that. Roel did find a real problem, but
we need to make sure the fix doesn't break stuff since it appears the
rate code is more complicated than we thought.
Dan
> 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@xxxxxxxxx>
> > Tested-by: Daniel Mack <daniel@xxxxxxxx>
> > ---
> > > 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;
> >
>
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
