On Wed, Nov 02, 2022 at 12:57:05PM -0700, coverity-bot wrote: > Hello! > > This is an experimental semi-automated report about issues detected by > Coverity from a scan of next-20221102 as part of the linux-next scan project: > https://scan.coverity.com/projects/linux-next-weekly-scan > > You're getting this email because you were associated with the identified > lines of code (noted below) that were touched by commits: > > Mon Oct 31 13:37:57 2022 -0300 > 91b4be750274 ("iommufd: Data structure to provide IOVA to PFN mapping") > > Coverity reported the following: > > *** CID 1527090: Null pointer dereferences (REVERSE_INULL) > /drivers/iommu/iommufd/io_pagetable.c: 417 in iopt_map_user_pages() > 411 list_add(&elm.next, &pages_list); > 412 > 413 rc = iopt_map_pages(iopt, &pages_list, length, iova, iommu_prot, flags); > 414 if (rc) { > 415 if (elm.area) > 416 iopt_abort_area(elm.area); > vvv CID 1527090: Null pointer dereferences (REVERSE_INULL) > vvv Null-checking "elm.pages" suggests that it may be null, but it has already been dereferenced on all paths leading to the check. > 417 if (elm.pages) > 418 iopt_put_pages(elm.pages); > 419 return rc; > 420 } > 421 return 0; > 422 } > > If this is a false positive, please let us know so we can mark it as > such, or teach the Coverity rules to be smarter. If not, please make > sure fixes get into linux-next. :) For patches fixing this, please > include these lines (but double-check the "Fixes" first): > > Reported-by: coverity-bot <keescook+coverity-bot@xxxxxxxxxxxx> > Addresses-Coverity-ID: 1527090 ("Null pointer dereferences") > Fixes: 91b4be750274 ("iommufd: Data structure to provide IOVA to PFN mapping") > > This looks like the earlier "if (IS_ERR(elm.pages))" should use > IS_ERR_OR_NULL() ? No, that exits the function, can't get to this code with a NULL or ERR from iopt_alloc_pages() It becomes NULL because one of the things iopt_map_pages() is to make it NULL. I think it is complaining that on the rc!=0 path of iopt_map_pages() it never makes it NULL. But I think I will leave this alone as the protocol to destroy an elm is to do all these tests, and an open coded stack creation of a elm shouldn't be asymetric with the heap allocated elms. Thanks, Jason
