| Message ID | 20211118142124.526901-1-arnd@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | fe785f56ad5886c08d1cadd9e8b4e1ff6a1866f6 |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | [v3] iwlwifi: pcie: fix constant-conversion warning | expand |
On Thu, Nov 18, 2021 at 03:21:02PM +0100, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > gcc-11 points out a potential issue with integer overflow when > the iwl_dev_info_table[] array is empty: > > drivers/net/wireless/intel/iwlwifi/pcie/drv.c:1344:42: error: implicit conversion from 'unsigned long' to 'int' changes value from 18446744073709551615 to -1 [-Werror,-Wconstant-conversion] > for (i = ARRAY_SIZE(iwl_dev_info_table) - 1; i >= 0; i--) { > ~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~ For what it's worth, I do see this warning with Clang when both CONFIG_IWLDVM and CONFIG_IWLMVM are disabled and looking through the GCC warning docs [1], I do not see a -Wconstant-conversion option? Maybe there is another warning that is similar but that warning right there appears to have come from clang, as it matches mine exactly. drivers/net/wireless/intel/iwlwifi/pcie/drv.c:1344:42: error: implicit conversion from 'unsigned long' to 'int' changes value from 18446744073709551615 to -1 [-Werror,-Wconstant-conversion] for (i = ARRAY_SIZE(iwl_dev_info_table) - 1; i >= 0; i--) { ~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~ 1 error generated. [1]: https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html > This is still harmless, as the loop correctly terminates, but adding > an extra range check makes that obvious to both readers and to the > compiler. > > Fixes: 3f7320428fa4 ("iwlwifi: pcie: simplify iwl_pci_find_dev_info()") > Reported-by: kernel test robot <lkp@intel.com> > Cc: Nick Desaulniers <ndesaulniers@google.com> > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Regardless of above, this resolves the warning for clang. Reviewed-by: Nathan Chancellor <nathan@kernel.org> > --- > Changes in v3: > - make it actually work with gcc-11 > - fix commit message s/clang/gcc-11/ > > Changes in v2: > - replace int cast with a range check > --- > drivers/net/wireless/intel/iwlwifi/pcie/drv.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c > index c574f041f096..395e328c6a07 100644 > --- a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c > +++ b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c > @@ -1339,9 +1339,13 @@ iwl_pci_find_dev_info(u16 device, u16 subsystem_device, > u16 mac_type, u8 mac_step, > u16 rf_type, u8 cdb, u8 rf_id, u8 no_160, u8 cores) > { > + int num_devices = ARRAY_SIZE(iwl_dev_info_table); > int i; > > - for (i = ARRAY_SIZE(iwl_dev_info_table) - 1; i >= 0; i--) { > + if (!num_devices) > + return NULL; > + > + for (i = num_devices - 1; i >= 0; i--) { > const struct iwl_dev_info *dev_info = &iwl_dev_info_table[i]; > > if (dev_info->device != (u16)IWL_CFG_ANY && > -- > 2.29.2 >
On Thu, Nov 18, 2021 at 5:03 PM Nathan Chancellor <nathan@kernel.org> wrote: > > On Thu, Nov 18, 2021 at 03:21:02PM +0100, Arnd Bergmann wrote: > > From: Arnd Bergmann <arnd@arndb.de> > > > > gcc-11 points out a potential issue with integer overflow when > > the iwl_dev_info_table[] array is empty: > > > > drivers/net/wireless/intel/iwlwifi/pcie/drv.c:1344:42: error: implicit conversion from 'unsigned long' to 'int' changes value from 18446744073709551615 to -1 [-Werror,-Wconstant-conversion] > > for (i = ARRAY_SIZE(iwl_dev_info_table) - 1; i >= 0; i--) { > > ~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~ > > For what it's worth, I do see this warning with Clang when both > CONFIG_IWLDVM and CONFIG_IWLMVM are disabled and looking through the GCC > warning docs [1], I do not see a -Wconstant-conversion option? Maybe > there is another warning that is similar but that warning right there > appears to have come from clang, as it matches mine exactly. > > drivers/net/wireless/intel/iwlwifi/pcie/drv.c:1344:42: error: implicit conversion from 'unsigned long' to 'int' changes value from 18446744073709551615 to -1 [-Werror,-Wconstant-conversion] > for (i = ARRAY_SIZE(iwl_dev_info_table) - 1; i >= 0; i--) { > ~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~ > 1 error generated. > > [1]: https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html Ok, got it: it turns out this warning /also/ happens with gcc-11 and the initial changelog was the one for matching the clang warning. This is the gcc output, which is very similar but has a different warning option. drivers/net/wireless/intel/iwlwifi/pcie/drv.c: In function 'iwl_pci_find_dev_info': include/linux/kernel.h:46:25: error: overflow in conversion from 'long unsigned int' to 'int' changes value from '18446744073709551615' to '-1' [-Werror=overflow] 46 | #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr)) | ^ drivers/net/wireless/intel/iwlwifi/pcie/drv.c:1344:18: note: in expansion of macro 'ARRAY_SIZE' 1344 | for (i = ARRAY_SIZE(iwl_dev_info_table) - 1; i >= 0; i--) { My v2 patch only addressed the clang warning, while v3 works with both gcc and clang. I can send a v4 if I should update the changelog again to explain that, but I suppose it's still close enough. Arnd
Arnd Bergmann <arnd@kernel.org> wrote: > From: Arnd Bergmann <arnd@arndb.de> > > Both gcc-11 and clang point out a potential issue with integer overflow when > the iwl_dev_info_table[] array is empty. This is what clang warns: > > drivers/net/wireless/intel/iwlwifi/pcie/drv.c:1344:42: error: implicit conversion from 'unsigned long' to 'int' changes value from 18446744073709551615 to -1 [-Werror,-Wconstant-conversion] > for (i = ARRAY_SIZE(iwl_dev_info_table) - 1; i >= 0; i--) { > ~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~ > > This is still harmless, as the loop correctly terminates, but adding > an extra range check makes that obvious to both readers and to the > compiler. > > Fixes: 3f7320428fa4 ("iwlwifi: pcie: simplify iwl_pci_find_dev_info()") > Reported-by: kernel test robot <lkp@intel.com> > Cc: Nick Desaulniers <ndesaulniers@google.com> > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > Reviewed-by: Nathan Chancellor <nathan@kernel.org> Patch applied to wireless-drivers.git, thanks. fe785f56ad58 iwlwifi: pcie: fix constant-conversion warning
diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c index c574f041f096..395e328c6a07 100644 --- a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c +++ b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c @@ -1339,9 +1339,13 @@ iwl_pci_find_dev_info(u16 device, u16 subsystem_device, u16 mac_type, u8 mac_step, u16 rf_type, u8 cdb, u8 rf_id, u8 no_160, u8 cores) { + int num_devices = ARRAY_SIZE(iwl_dev_info_table); int i; - for (i = ARRAY_SIZE(iwl_dev_info_table) - 1; i >= 0; i--) { + if (!num_devices) + return NULL; + + for (i = num_devices - 1; i >= 0; i--) { const struct iwl_dev_info *dev_info = &iwl_dev_info_table[i]; if (dev_info->device != (u16)IWL_CFG_ANY &&