| Message ID | 1439512532-7901-1-git-send-email-adrien+dev@schischi.me |
|---|---|
| State | Awaiting Upstream, archived |
| Delegated to: | David Miller |
| Headers | show |
Hi, On 08/14/2015 03:36 AM, Adrien Schildknecht wrote: > Both loops of this function compare data from the 'chan' array and then > check if the index is valid. > > The 2 conditions should be inverted to avoid an out-of-bounds access. > Was that found by a static analyzer or any other automated tool, or was that the result of your very careful review? > Signed-off-by: Adrien Schildknecht <adrien+dev@schischi.me> > --- > drivers/net/wireless/iwlwifi/iwl-eeprom-parse.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/wireless/iwlwifi/iwl-eeprom-parse.c b/drivers/net/wireless/iwlwifi/iwl-eeprom-parse.c > index 21302b6..acc3d18 100644 > --- a/drivers/net/wireless/iwlwifi/iwl-eeprom-parse.c > +++ b/drivers/net/wireless/iwlwifi/iwl-eeprom-parse.c > @@ -713,12 +713,12 @@ int iwl_init_sband_channels(struct iwl_nvm_data *data, > struct ieee80211_channel *chan = &data->channels[0]; > int n = 0, idx = 0; > > - while (chan->band != band && idx < n_channels) > + while (idx < n_channels && chan->band != band) > chan = &data->channels[++idx]; > > sband->channels = &data->channels[idx]; > > - while (chan->band == band && idx < n_channels) { > + while (idx < n_channels && chan->band == band) { > chan = &data->channels[++idx]; > n++; > } > Looks fine - I'll pick it up. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, > On 08/14/2015 03:36 AM, Adrien Schildknecht wrote: > > Both loops of this function compare data from the 'chan' array and > > then check if the index is valid. > > > > The 2 conditions should be inverted to avoid an out-of-bounds > > access. > > > > Was that found by a static analyzer or any other automated tool, or > was that the result of your very careful review? The error has been reported by KASan: ================================================================== BUG: KASan: out of bounds access in iwl_init_sband_channels+0x207/0x260 [iwlwifi] at addr ffff8800c2d0aac8 Read of size 4 by task modprobe/329 ==================================================================
Adrien Schildknecht <adrien+dev@schischi.me> writes: > Hi, > >> On 08/14/2015 03:36 AM, Adrien Schildknecht wrote: >> > Both loops of this function compare data from the 'chan' array and >> > then check if the index is valid. >> > >> > The 2 conditions should be inverted to avoid an out-of-bounds >> > access. >> > >> >> Was that found by a static analyzer or any other automated tool, or >> was that the result of your very careful review? > > The error has been reported by KASan: > ================================================================== > BUG: KASan: out of bounds access in iwl_init_sband_channels+0x207/0x260 [iwlwifi] at addr ffff8800c2d0aac8 > Read of size 4 by task modprobe/329 > ================================================================== Always try to add information like this to the commit log, it's very useful.
diff --git a/drivers/net/wireless/iwlwifi/iwl-eeprom-parse.c b/drivers/net/wireless/iwlwifi/iwl-eeprom-parse.c index 21302b6..acc3d18 100644 --- a/drivers/net/wireless/iwlwifi/iwl-eeprom-parse.c +++ b/drivers/net/wireless/iwlwifi/iwl-eeprom-parse.c @@ -713,12 +713,12 @@ int iwl_init_sband_channels(struct iwl_nvm_data *data, struct ieee80211_channel *chan = &data->channels[0]; int n = 0, idx = 0; - while (chan->band != band && idx < n_channels) + while (idx < n_channels && chan->band != band) chan = &data->channels[++idx]; sband->channels = &data->channels[idx]; - while (chan->band == band && idx < n_channels) { + while (idx < n_channels && chan->band == band) { chan = &data->channels[++idx]; n++; }
Both loops of this function compare data from the 'chan' array and then check if the index is valid. The 2 conditions should be inverted to avoid an out-of-bounds access. Signed-off-by: Adrien Schildknecht <adrien+dev@schischi.me> --- drivers/net/wireless/iwlwifi/iwl-eeprom-parse.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)