Hello, On Thu, 2024-04-04 at 09:29 +0300, Dan Carpenter wrote: > Hi Philipp, > > kernel test robot noticed the following build warnings: > > https://git-scm.com/docs/git-format-patch#_base_tree_information] > > url: > https://github.com/intel-lab-lkp/linux/commits/Philipp-Stanner/PCI-Add-new-set-of-devres-functions/20240403-160932 > base: > https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next > patch link: > https://lore.kernel.org/r/20240403080712.13986-5-pstanner%40redhat.com > patch subject: [PATCH v5 02/10] PCI: Deprecate iomap-table functions > config: i386-randconfig-141-20240404 > (https://download.01.org/0day-ci/archive/20240404/202404040920.QIxhNe > Mu-lkp@xxxxxxxxx/config) > compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0 > > If you fix the issue in a separate patch/commit (i.e. not just a new > version of > the same patch/commit), kindly add following tags > > Reported-by: kernel test robot <lkp@xxxxxxxxx> > > Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx> > > Closes: > > https://lore.kernel.org/r/202404040920.QIxhNeMu-lkp@xxxxxxxxx/ > > smatch warnings: > drivers/pci/devres.c:897 pcim_iomap_regions_request_all() error: we > previously assumed 'legacy_iomap_table' could be null (see line 894) > > vim +/legacy_iomap_table +897 drivers/pci/devres.c > > acc2364fe66106 Philipp Stanner 2024-01-31 865 int > pcim_iomap_regions_request_all(struct pci_dev *pdev, int mask, > acc2364fe66106 Philipp Stanner 2024-01-31 > 866 const char *name) > acc2364fe66106 Philipp Stanner 2024-01-31 867 { > 34e90b966504f3 Philipp Stanner 2024-04-03 868 short bar; > 34e90b966504f3 Philipp Stanner 2024-04-03 869 int ret; > 34e90b966504f3 Philipp Stanner 2024-04-03 870 void __iomem > **legacy_iomap_table; > acc2364fe66106 Philipp Stanner 2024-01-31 871 > 34e90b966504f3 Philipp Stanner 2024-04-03 872 ret = > pcim_request_all_regions(pdev, name); > 34e90b966504f3 Philipp Stanner 2024-04-03 873 if (ret != 0) > 34e90b966504f3 Philipp Stanner 2024-04-03 > 874 return ret; > acc2364fe66106 Philipp Stanner 2024-01-31 875 > 34e90b966504f3 Philipp Stanner 2024-04-03 876 for (bar = 0; > bar < PCI_STD_NUM_BARS; bar++) { > 34e90b966504f3 Philipp Stanner 2024-04-03 877 if > (!mask_contains_bar(mask, bar)) > 34e90b966504f3 Philipp Stanner 2024-04-03 > 878 continue; > 34e90b966504f3 Philipp Stanner 2024-04-03 879 if > (!pcim_iomap(pdev, bar, 0)) > 34e90b966504f3 Philipp Stanner 2024-04-03 > 880 goto err; > 34e90b966504f3 Philipp Stanner 2024-04-03 881 } > 34e90b966504f3 Philipp Stanner 2024-04-03 882 > 34e90b966504f3 Philipp Stanner 2024-04-03 883 return 0; > 34e90b966504f3 Philipp Stanner 2024-04-03 884 > 34e90b966504f3 Philipp Stanner 2024-04-03 885 err: > 34e90b966504f3 Philipp Stanner 2024-04-03 886 /* > 34e90b966504f3 Philipp Stanner 2024-04-03 887 * Here it > gets tricky: pcim_iomap() above has most likely > 34e90b966504f3 Philipp Stanner 2024-04-03 888 * failed > because it got an OOM when trying to create the > 34e90b966504f3 Philipp Stanner 2024-04-03 889 * legacy- > table. > 34e90b966504f3 Philipp Stanner 2024-04-03 890 * We check > here if that has happened. If not, pcim_iomap() > 34e90b966504f3 Philipp Stanner 2024-04-03 891 * must have > failed because of EINVAL. > 34e90b966504f3 Philipp Stanner 2024-04-03 892 */ > 34e90b966504f3 Philipp Stanner 2024-04-03 > 893 legacy_iomap_table = (void __iomem > **)pcim_iomap_table(pdev); > 34e90b966504f3 Philipp Stanner 2024-04-03 @894 ret = > legacy_iomap_table ? -EINVAL : -ENOMEM; > > ^^^^^^^^^^^^^^^^^^ > Check for NULL > > 34e90b966504f3 Philipp Stanner 2024-04-03 895 > 34e90b966504f3 Philipp Stanner 2024-04-03 896 while (--bar > >= 0) > 34e90b966504f3 Philipp Stanner 2024-04-03 > @897 pcim_iounmap(pdev, legacy_iomap_table[bar]); > > ^^^^^^^^^^^^^^^^^^ > Unchecked dereference I think this is fine because `bar` can only be larger 0 if at least one mapping has been created, thus, when it was possible to create legacy_iomap_table, which is then valid. So the second for-loop only executes when it's not NULL. I guess we could silence the warning by doing ret = bar > 0 ? -EINVAL : -ENOMEM; Would even save one line of code P. > > 34e90b966504f3 Philipp Stanner 2024-04-03 898 > 34e90b966504f3 Philipp Stanner 2024-04-03 > 899 pcim_release_all_regions(pdev); > 34e90b966504f3 Philipp Stanner 2024-04-03 900 > 34e90b966504f3 Philipp Stanner 2024-04-03 901 return ret; > acc2364fe66106 Philipp Stanner 2024-01-31 902 } >
