| Date | Wed, 29 Jan 2020 23:47:19 +0100 | From | Pavel Machek <> | Subject | Re: [PATCH 4.19 63/92] libertas: Fix two buffer overflows at parsing bss descriptor |
| |
Hi!
> From: Wen Huang <huangwenabc@gmail.com> > > commit e5e884b42639c74b5b57dc277909915c0aefc8bb upstream.
> --- a/drivers/net/wireless/marvell/libertas/cfg.c > +++ b/drivers/net/wireless/marvell/libertas/cfg.c > @@ -1717,6 +1721,9 @@ static int lbs_ibss_join_existing(struct > struct cmd_ds_802_11_ad_hoc_join cmd; > u8 preamble = RADIO_PREAMBLE_SHORT; > int ret = 0; > + int hw, i; > + u8 rates_max; > + u8 *rates; > > /* TODO: set preamble based on scan result */ > ret = lbs_set_radio(priv, preamble, 1); > @@ -1775,9 +1782,12 @@ static int lbs_ibss_join_existing(struct > if (!rates_eid) { > lbs_add_rates(cmd.bss.rates); > } else { > - int hw, i; > - u8 rates_max = rates_eid[1]; > - u8 *rates = cmd.bss.rates; > + rates_max = rates_eid[1];
I believe original version (with variables being local to the else) was better.
> + if (rates_max > MAX_RATES) { > + lbs_deb_join("invalid rates"); > + goto out; > + } > + rates = cmd.bss.rates;
"goto out" goes to "return ret". ret will be 0 at this point, so this will return success. I don't think that's right.
Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [unhandled content-type:application/pgp-signature] |