1. Patchew
  2. QEMU
  3. View series

[PATCH] vfio/migration: Fix page size calculation

Zhenzhong Duan posted 1 patch 3 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260116060315.65723-1-zhenzhong.duan@intel.com
Maintainers: Alex Williamson <alex@shazbot.org>, "Cédric Le Goater" <clg@redhat.com>
hw/vfio/migration.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] vfio/migration: Fix page size calculation
Posted by Zhenzhong Duan 3 weeks ago 
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 <clg@redhat.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 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;
 
     return current_machine->ram_size > max_size;
-- 
2.47.1
Re: [PATCH] vfio/migration: Fix page size calculation
Posted by Peter Maydell 2 weeks, 1 day ago 
On Fri, 16 Jan 2026 at 06:03, Zhenzhong Duan <zhenzhong.duan@intel.com> 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 <clg@redhat.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>  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.

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.

thanks
-- PMM
RE: [PATCH] vfio/migration: Fix page size calculation
Posted by Duan, Zhenzhong 2 weeks ago 

>-----Original Message-----
>From: Peter Maydell <peter.maydell@linaro.org>
>Subject: Re: [PATCH] vfio/migration: Fix page size calculation
>
>On Fri, 16 Jan 2026 at 06:03, Zhenzhong Duan <zhenzhong.duan@intel.com>
>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 <clg@redhat.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>>  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😊

Thanks
Zhenzhong
Re: [PATCH] vfio/migration: Fix page size calculation
Posted by Peter Maydell 2 weeks ago 
On Fri, 23 Jan 2026 at 03:38, Duan, Zhenzhong <zhenzhong.duan@intel.com> wrote:
>
>
>
> >-----Original Message-----
> >From: Peter Maydell <peter.maydell@linaro.org>
> >Subject: Re: [PATCH] vfio/migration: Fix page size calculation
> >
> >On Fri, 16 Jan 2026 at 06:03, Zhenzhong Duan <zhenzhong.duan@intel.com>
> >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 <clg@redhat.com>
> >> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> >> ---
> >>  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
Re: [PATCH] vfio/migration: Fix page size calculation
Posted by Cédric Le Goater 3 weeks ago 
On 1/16/26 07:03, Zhenzhong Duan 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 <clg@redhat.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>   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;
>   
>       return current_machine->ram_size > max_size;

Applied to vfio-next.

Thanks,

C.
Re: [PATCH] vfio/migration: Fix page size calculation
Posted by Cédric Le Goater 3 weeks ago 
On 1/16/26 07:03, Zhenzhong Duan 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 <clg@redhat.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>   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;
>   
>       return current_machine->ram_size > max_size;

Reviewed-by: Cédric Le Goater <clg@redhat.com>


Thanks,

C.

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.