[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v4 01/21] AMD/IOMMU: correct potentially-UB shifts
On 03.05.2022 12:10, Roger Pau Monné wrote: > On Mon, Apr 25, 2022 at 10:30:33AM +0200, Jan Beulich wrote: >> Recent changes (likely 5fafa6cf529a ["AMD/IOMMU: have callers specify >> the target level for page table walks"]) have made Coverity notice a >> shift count in iommu_pde_from_dfn() which might in theory grow too >> large. While this isn't a problem in practice, address the concern >> nevertheless to not leave dangling breakage in case very large >> superpages would be enabled at some point. >> >> Coverity ID: 1504264 >> >> While there also address a similar issue in set_iommu_ptes_present(). >> It's not clear to me why Coverity hasn't spotted that one. >> >> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx> > > Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx> Thanks. >> --- a/xen/drivers/passthrough/amd/iommu_map.c >> +++ b/xen/drivers/passthrough/amd/iommu_map.c >> @@ -89,11 +89,11 @@ static unsigned int set_iommu_ptes_prese >> bool iw, bool ir) >> { >> union amd_iommu_pte *table, *pde; >> - unsigned int page_sz, flush_flags = 0; >> + unsigned long page_sz = 1UL << (PTE_PER_TABLE_SHIFT * (pde_level - 1)); > > Seeing the discussion from Andrews reply, nr_pages might be more > appropriate while still quite short. Yes and no - it then would be ambiguous as to what size pages are meant. > I'm not making my Rb conditional to that change though. Good, thanks. But I guess I'm still somewhat stuck unless hearing back from Andrew (although one might not count a conditional R-b as a "pending objection"). I'll give him a few more days, but I continue to think this ought to be a separate change (if renaming is really needed in the 1st place) ... Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |