Fix bug inside replay_by_populated_state() that forgets to initialize
MemoryRegionSection field offset_within_address_space. Follow the
established pattern of calling memory_region_section_intersect_range()
to accomplish this task.

Prior to commit cc9c77f4ddf0 ("system/memory: implement
RamDiscardManager multi-source aggregation"),
replay_by_populated_state() had called
memory_region_section_intersect_range() in order to initialize
interdependent fields offset_within_address_space, offset_within_region,
and size together, as shown below.

    s->offset_within_address_space += start - s->offset_within_region;
    s->offset_within_region = start;
    s->size = int128_sub(end, int128_make64(start));

cc9c77f4ddf0 reimplements replay_by_populated_state() initializing the
fields of the given MemoryRegionSection instance by hand instead of via
memory_region_section_intersect_range(). In doing so, it leaves
offset_within_address_space uninitialized for some reason, as you can
see below.

    MemoryRegionSection subsection = {
        .mr = section->mr,
        .offset_within_region = offset,
        .size = int128_make64(MIN(granularity, end_offset - offset)),
    };

Consequently offset_within_address_space defaults to GPA 0x0, which is
incorrect. For example, on s390x, base RAM begins at GPA 0x0 and it is
problematic to report that a virtio-iommu MR section lives there
instead.

cc9c77f4ddf0 deliberately calls memory_region_section_intersect_range()
from other related code paths inside the same file, namely
replay_source_by_state() and rdl_populate_cb()/rdl_discard_cb(). It is
unclear why the new replay_by_populated_state() implementation does not
conform to this same pattern.

The effects of the bug include qemu crashes on multiple architectures.
The following assertion failure occurs when driving the
guest_phys_blocks_append() code path, for guests with virtio-mem device
that has some memory plugged.

    DBG: guest_phys_block_add_section: predecessor->target_end=280000000 target_start=0
    **
    ERROR:../system/memory_mapping.c:222:guest_phys_block_add_section: assertion failed: (predecessor->target_end <= target_start)
    Bail out! ERROR:../system/memory_mapping.c:222:guest_phys_block_add_section: assertion failed: (predecessor->target_end <= target_start)
    2026-09-11 16:03:57.405+0000: shutting down, reason=crashed

This crash can be triggered on x86 via the dump-guest-memory QMP
command. The same crash can be triggered on s390x by restoring VM State
that has been migrated to a local file. (I used libvirt to manage this
migration restore operation, namely command virsh managedsave then virsh
start.) Applying the fix resolved the crash on both platforms.

Fixes: cc9c77f4ddf0 ("system/memory: implement RamDiscardManager multi-source aggregation")
Reported-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Cam Miller <cam@linux.ibm.com>
---
 system/ram-discard-manager.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/system/ram-discard-manager.c b/system/ram-discard-manager.c
index 4e8816e5a2..e9a609e5cd 100644
--- a/system/ram-discard-manager.c
+++ b/system/ram-discard-manager.c
@@ -238,14 +238,15 @@ static int replay_by_populated_state(const RamDiscardManager *rdm,
             }
         } else {
             if (in_run) {
-                MemoryRegionSection run_section = {
-                    .mr = section->mr,
-                    .offset_within_region = run_start,
-                    .size = int128_make64(offset - run_start),
-                };
-                ret = replay_fn(&run_section, user_opaque);
-                if (ret) {
-                    return ret;
+                MemoryRegionSection run_section = *section;
+
+                if (memory_region_section_intersect_range(&run_section,
+                                                          run_start,
+                                                          offset - run_start)) {
+                    ret = replay_fn(&run_section, user_opaque);
+                    if (ret) {
+                        return ret;
+                    }
                 }
                 in_run = false;
             }
@@ -257,12 +258,12 @@ static int replay_by_populated_state(const RamDiscardManager *rdm,
     }
 
     if (in_run) {
-        MemoryRegionSection run_section = {
-            .mr = section->mr,
-            .offset_within_region = run_start,
-            .size = int128_make64(end_offset - run_start),
-        };
-        ret = replay_fn(&run_section, user_opaque);
+        MemoryRegionSection run_section = *section;
+
+        if (memory_region_section_intersect_range(&run_section, run_start,
+                                                  end_offset - run_start)) {
+            ret = replay_fn(&run_section, user_opaque);
+        }
     }
 
     return ret;

---
base-commit: 2242ae1f7bdcf76e78cf8a987118952fc6c9a469
change-id: 20260914-vmem_fix-c80d75571aa6

Best regards,
-- 
Cam Miller <cam@linux.ibm.com>

Hi

On Mon, Sep 14, 2026 at 7:35 PM Cam Miller <cam@linux.ibm.com> wrote:
>
> Fix bug inside replay_by_populated_state() that forgets to initialize
> MemoryRegionSection field offset_within_address_space. Follow the
> established pattern of calling memory_region_section_intersect_range()
> to accomplish this task.
>
> Prior to commit cc9c77f4ddf0 ("system/memory: implement
> RamDiscardManager multi-source aggregation"),
> replay_by_populated_state() had called
> memory_region_section_intersect_range() in order to initialize
> interdependent fields offset_within_address_space, offset_within_region,
> and size together, as shown below.
>
>     s->offset_within_address_space += start - s->offset_within_region;
>     s->offset_within_region = start;
>     s->size = int128_sub(end, int128_make64(start));
>
> cc9c77f4ddf0 reimplements replay_by_populated_state() initializing the
> fields of the given MemoryRegionSection instance by hand instead of via
> memory_region_section_intersect_range(). In doing so, it leaves
> offset_within_address_space uninitialized for some reason, as you can
> see below.
>
>     MemoryRegionSection subsection = {
>         .mr = section->mr,
>         .offset_within_region = offset,
>         .size = int128_make64(MIN(granularity, end_offset - offset)),
>     };
>
> Consequently offset_within_address_space defaults to GPA 0x0, which is
> incorrect. For example, on s390x, base RAM begins at GPA 0x0 and it is
> problematic to report that a virtio-iommu MR section lives there
> instead.
>
> cc9c77f4ddf0 deliberately calls memory_region_section_intersect_range()
> from other related code paths inside the same file, namely
> replay_source_by_state() and rdl_populate_cb()/rdl_discard_cb(). It is
> unclear why the new replay_by_populated_state() implementation does not
> conform to this same pattern.
>
> The effects of the bug include qemu crashes on multiple architectures.
> The following assertion failure occurs when driving the
> guest_phys_blocks_append() code path, for guests with virtio-mem device
> that has some memory plugged.
>
>     DBG: guest_phys_block_add_section: predecessor->target_end=280000000 target_start=0
>     **
>     ERROR:../system/memory_mapping.c:222:guest_phys_block_add_section: assertion failed: (predecessor->target_end <= target_start)
>     Bail out! ERROR:../system/memory_mapping.c:222:guest_phys_block_add_section: assertion failed: (predecessor->target_end <= target_start)
>     2026-09-11 16:03:57.405+0000: shutting down, reason=crashed
>
> This crash can be triggered on x86 via the dump-guest-memory QMP
> command. The same crash can be triggered on s390x by restoring VM State
> that has been migrated to a local file. (I used libvirt to manage this
> migration restore operation, namely command virsh managedsave then virsh
> start.) Applying the fix resolved the crash on both platforms.
>
> Fixes: cc9c77f4ddf0 ("system/memory: implement RamDiscardManager multi-source aggregation")
> Reported-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
> Signed-off-by: Cam Miller <cam@linux.ibm.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

thanks

> ---
>  system/ram-discard-manager.c | 29 +++++++++++++++--------------
>  1 file changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/system/ram-discard-manager.c b/system/ram-discard-manager.c
> index 4e8816e5a2..e9a609e5cd 100644
> --- a/system/ram-discard-manager.c
> +++ b/system/ram-discard-manager.c
> @@ -238,14 +238,15 @@ static int replay_by_populated_state(const RamDiscardManager *rdm,
>              }
>          } else {
>              if (in_run) {
> -                MemoryRegionSection run_section = {
> -                    .mr = section->mr,
> -                    .offset_within_region = run_start,
> -                    .size = int128_make64(offset - run_start),
> -                };
> -                ret = replay_fn(&run_section, user_opaque);
> -                if (ret) {
> -                    return ret;
> +                MemoryRegionSection run_section = *section;
> +
> +                if (memory_region_section_intersect_range(&run_section,
> +                                                          run_start,
> +                                                          offset - run_start)) {
> +                    ret = replay_fn(&run_section, user_opaque);
> +                    if (ret) {
> +                        return ret;
> +                    }
>                  }
>                  in_run = false;
>              }
> @@ -257,12 +258,12 @@ static int replay_by_populated_state(const RamDiscardManager *rdm,
>      }
>
>      if (in_run) {
> -        MemoryRegionSection run_section = {
> -            .mr = section->mr,
> -            .offset_within_region = run_start,
> -            .size = int128_make64(end_offset - run_start),
> -        };
> -        ret = replay_fn(&run_section, user_opaque);
> +        MemoryRegionSection run_section = *section;
> +
> +        if (memory_region_section_intersect_range(&run_section, run_start,
> +                                                  end_offset - run_start)) {
> +            ret = replay_fn(&run_section, user_opaque);
> +        }
>      }
>
>      return ret;
>
> ---
> base-commit: 2242ae1f7bdcf76e78cf8a987118952fc6c9a469
> change-id: 20260914-vmem_fix-c80d75571aa6
>
> Best regards,
> --
> Cam Miller <cam@linux.ibm.com>
>
>


-- 
Marc-André Lureau

On Mon, Sep 14, 2026 at 07:54:44PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Mon, Sep 14, 2026 at 7:35 PM Cam Miller <cam@linux.ibm.com> wrote:
> >
> > Fix bug inside replay_by_populated_state() that forgets to initialize
> > MemoryRegionSection field offset_within_address_space. Follow the
> > established pattern of calling memory_region_section_intersect_range()
> > to accomplish this task.
> >
> > Prior to commit cc9c77f4ddf0 ("system/memory: implement
> > RamDiscardManager multi-source aggregation"),
> > replay_by_populated_state() had called
> > memory_region_section_intersect_range() in order to initialize
> > interdependent fields offset_within_address_space, offset_within_region,
> > and size together, as shown below.
> >
> >     s->offset_within_address_space += start - s->offset_within_region;
> >     s->offset_within_region = start;
> >     s->size = int128_sub(end, int128_make64(start));
> >
> > cc9c77f4ddf0 reimplements replay_by_populated_state() initializing the
> > fields of the given MemoryRegionSection instance by hand instead of via
> > memory_region_section_intersect_range(). In doing so, it leaves
> > offset_within_address_space uninitialized for some reason, as you can
> > see below.
> >
> >     MemoryRegionSection subsection = {
> >         .mr = section->mr,
> >         .offset_within_region = offset,
> >         .size = int128_make64(MIN(granularity, end_offset - offset)),
> >     };
> >
> > Consequently offset_within_address_space defaults to GPA 0x0, which is
> > incorrect. For example, on s390x, base RAM begins at GPA 0x0 and it is
> > problematic to report that a virtio-iommu MR section lives there
> > instead.
> >
> > cc9c77f4ddf0 deliberately calls memory_region_section_intersect_range()
> > from other related code paths inside the same file, namely
> > replay_source_by_state() and rdl_populate_cb()/rdl_discard_cb(). It is
> > unclear why the new replay_by_populated_state() implementation does not
> > conform to this same pattern.
> >
> > The effects of the bug include qemu crashes on multiple architectures.
> > The following assertion failure occurs when driving the
> > guest_phys_blocks_append() code path, for guests with virtio-mem device
> > that has some memory plugged.
> >
> >     DBG: guest_phys_block_add_section: predecessor->target_end=280000000 target_start=0
> >     **
> >     ERROR:../system/memory_mapping.c:222:guest_phys_block_add_section: assertion failed: (predecessor->target_end <= target_start)
> >     Bail out! ERROR:../system/memory_mapping.c:222:guest_phys_block_add_section: assertion failed: (predecessor->target_end <= target_start)
> >     2026-09-11 16:03:57.405+0000: shutting down, reason=crashed
> >
> > This crash can be triggered on x86 via the dump-guest-memory QMP
> > command. The same crash can be triggered on s390x by restoring VM State
> > that has been migrated to a local file. (I used libvirt to manage this
> > migration restore operation, namely command virsh managedsave then virsh
> > start.) Applying the fix resolved the crash on both platforms.
> >
> > Fixes: cc9c77f4ddf0 ("system/memory: implement RamDiscardManager multi-source aggregation")
> > Reported-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
> > Signed-off-by: Cam Miller <cam@linux.ibm.com>
> 
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

queued, thanks.

-- 
Peter Xu