On Fri, 23 Jan 2026 at 03:38, Duan, Zhenzhong <[email protected]> wrote: > > > > >-----Original Message----- > >From: Peter Maydell <[email protected]> > >Subject: Re: [PATCH] vfio/migration: Fix page size calculation > > > >On Fri, 16 Jan 2026 at 06:03, Zhenzhong Duan <[email protected]> > >wrote: > >> > >> Coverity detected an issue of left shifting int by more than 31 bits > >> leading > >> to undefined behavior. > >> > >> In practice bcontainer->dirty_pgsizes always have some common page sizes > >> when dirty tracking is supported. > >> > >> Resolves: Coverity CID 1644186 > >> Resolves: Coverity CID 1644187 > >> Resolves: Coverity CID 1644188 > >> Fixes: 46c763311419 ("vfio/migration: Add migration blocker if VM memory > >is too large to cause unmap_bitmap failure"). > >> Suggested-by: Cédric Le Goater <[email protected]> > >> Signed-off-by: Zhenzhong Duan <[email protected]> > >> --- > >> hw/vfio/migration.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c > >> index f857dc25ed..b4695030c7 100644 > >> --- a/hw/vfio/migration.c > >> +++ b/hw/vfio/migration.c > >> @@ -1173,7 +1173,7 @@ static bool > >vfio_dirty_tracking_exceed_limit(VFIODevice *vbasedev) > >> * can also switch to use IOMMUFD backend if there is a need to > >migrate > >> * large VM. > >> */ > >> - page_size = 1 << ctz64(bcontainer->dirty_pgsizes); > >> + page_size = 1ULL << ctz64(bcontainer->dirty_pgsizes); > >> max_size = bcontainer->max_dirty_bitmap_size * BITS_PER_BYTE * > >page_size; > > > >This doesn't strictly speaking resolve CID 1644186, because > >what Coverity sees is that ctz64() returns 64 when dirty_pgsizes > >is zero. That is still UB even with the ULL suffix. > > Hmm, maybe, I didn't find easy way to verify it with Coverity test. > In fact, this code is guarded by below check: > > if (!bcontainer->dirty_pages_supported) { > return false; > } > > If the check pass, we are sure dirty_pgsizes has host page size bit set at > least: > > if (cap_mig->pgsize_bitmap & qemu_real_host_page_size()) { > bcontainer->dirty_pages_supported = true; > bcontainer->max_dirty_bitmap_size = cap_mig->max_dirty_bitmap_size; > bcontainer->dirty_pgsizes = cap_mig->pgsize_bitmap; > } > > > > >But if we are enforcing somewhere that dirty_pgsizes is > >never zero, then this is fine and we can mark that Coverity > >issue as a false-positive. > > May I ask how to mark it? > Maybe Coverity test is smart enough to not report in this case😊
It's usually not that clever. I'll mark the relevant issues as false-positive in the web UI. Thanks for confirming that there's a "definitely at least one bit set" check already. -- PMM
