[PATCH hci-8.0 0/2] migration/cpr: lock the cpr state #VSTOR-145529

[PATCH hci-8.0 0/2] migration/cpr: lock the cpr state #VSTOR-145529

[Denis V. Lunev]

A guest died with SIGSEGV in find_fd() after a routine libvirtd restart.
The fd registry in cpr_state has no lock, and since we started saving
chardev connection fds it is reached from more than one thread.

A socket chardev saves and drops its connection fd in
tcp_chr_new_client() and tcp_chr_free_connection(), which run in the
chardev's GMainContext. For libvirt's monitor that context belongs to the
QMP iothread, while the guest agent channels and the serial pty stay on
the main loop. All four are listening sockets, so a libvirtd restart
reconnects them at the same moment and two threads run QLIST_INSERT_HEAD
and QLIST_REMOVE on the same list.

The core on mhw0501 shows the result. cpr_state.fds.lh_first points at a
48 byte chunk whose malloc size field carries NON_MAIN_ARENA, so an
iothread allocated it, and it now holds a glib GSourceCallback for the
listener watch. The real list is intact and unreachable behind it:
vsock0, hostnet0 x2, charchannel1, charchannel0, charserial0 and
charmonitor, headed by a freed main arena entry with id 1, which is a
guest agent channel connection entry dropped by the main loop. Both
mutators are in the wreckage.

Taking the BQL in the chardev hooks is not an option. The chardev layer
never takes it, monitor_cleanup() stops the QMP iothread while holding
it, and the iothread exists precisely so out-of-band commands keep
working when the main thread will not release it. The lock belongs to the
registry.

The commit message is deliberately free of chardev and QLIST specifics so
that the same text serves upstream. Nothing upstream reaches this state
off the main thread, so upstream gets it as hardening rather than a fix,
against a rewritten cpr.c that keeps the fds in a GHashTable.

Patch 1 is the part that also goes upstream, so it stops at the fd
accessors and the vmstate walks. Patch 2 covers the ramblock list, which
upstream does not have. Splitting them keeps patch 1 a straight mirror of
the qemu-devel posting, which should make the next rebase onto a newer
upstream cheap.

The use-after-free read of elem->fd in the cpr_delete_fd() trace, which
this series originally carried as well, is already in as b6732b19c34.

Tested on dev007 against this base: tests/qemu-iotests migrate-cpr-exec
passes all 30 subtests, which covers the chardev serial, chardev server
unix and chardev-change hot-swap cases as well as the tap and vhost ones,
and tests/qtest/migration-test --full is 86 of 86 with
QEMU_TEST_FLAKY_TESTS=1 set, so mode/reboot, mode/transfer,
mode/transfer/defer and mode/exec all run.

Denis V. Lunev (2):
  migration/cpr: lock the saved fds #VSTOR-145529
  migration/cpr: lock the ramblocks #VSTOR-145529

 migration/cpr.c | 86 ++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 71 insertions(+), 15 deletions(-)


base-commit: 87f111d087cff956908ccb22daedb596b3734877
-- 
2.53.0

[PATCH hci-8.0 1/2] migration/cpr: lock the saved fds #VSTOR-145529

[Denis V. Lunev]

From: Denis V. Lunev <den@openvz.org>

The cpr state is a process wide registry of the resources that have to
survive an exec. Entries are added and dropped from realize and teardown
paths all over the tree, and a caller has no way to tell which thread the
next caller will run in.

The state carries no serialization of its own. It is safe today only
because its callers happen to hold the BQL, and that is a property of
where those callers live rather than anything the interface promises or
checks. A caller that is correct within its own subsystem can therefore
break this one from a distance: two that overlap corrupt the registry,
and the next lookup walks an entry that has already been freed.

Give the state a lock of its own instead of relying on that. Entries
move when a device is realized, when a resource is handed over, and when
a migration starts, so none of this is on a hot path and an uncontended
mutex costs nothing.

Take the lock in every fd accessor and across the save and load walks.
Holding it over the walk callbacks is safe because none of them comes
back through this interface. State that is not reached through these
accessors, such as the VFIO device list that hw/vfio manipulates
directly, stays outside it.

Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 migration/cpr.c | 60 ++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 50 insertions(+), 10 deletions(-)

diff --git a/migration/cpr.c b/migration/cpr.c
index f218e6a66d6..78cf430e386 100644
--- a/migration/cpr.c
+++ b/migration/cpr.c
@@ -9,6 +9,7 @@
 #include "qemu/error-report.h"
 #include "qapi/error.h"
 #include "qemu/error-report.h"
+#include "qemu/lockable.h"
 #include "hw/vfio/vfio-device.h"
 #include "migration/cpr.h"
 #include "migration/misc.h"
@@ -33,6 +34,14 @@
 
 CprState cpr_state;
 
+/* Callers are spread across the tree and need not share a thread. */
+static QemuMutex cpr_state_lock;
+
+static void __attribute__((constructor)) cpr_state_lock_init(void)
+{
+    qemu_mutex_init(&cpr_state_lock);
+}
+
 /****************************************************************************/
 
 typedef struct CprFd {
@@ -107,10 +116,22 @@ static CprFd *find_fd(CprFdList *head, const char *name, int id)
     return NULL;
 }
 
-void cpr_delete_fd(const char *name, int id)
+static int do_cpr_find_fd(const char *name, int id)
 {
     CprFd *elem = find_fd(&cpr_state.fds, name, id);
-    int fd = elem ? elem->fd : -1;
+
+    return elem ? elem->fd : -1;
+}
+
+void cpr_delete_fd(const char *name, int id)
+{
+    CprFd *elem;
+    int fd;
+
+    QEMU_LOCK_GUARD(&cpr_state_lock);
+
+    elem = find_fd(&cpr_state.fds, name, id);
+    fd = elem ? elem->fd : -1;
 
     if (elem) {
         QLIST_REMOVE(elem, next);
@@ -125,6 +146,8 @@ void cpr_delete_fd_all(const char *name)
 {
     CprFd *elem, *next_elem;
 
+    QEMU_LOCK_GUARD(&cpr_state_lock);
+
     QLIST_FOREACH_SAFE(elem, &cpr_state.fds, next, next_elem) {
         if (!strcmp(elem->name, name)) {
             QLIST_REMOVE(elem, next);
@@ -136,18 +159,22 @@ void cpr_delete_fd_all(const char *name)
 
 int cpr_find_fd(const char *name, int id)
 {
-    CprFd *elem = find_fd(&cpr_state.fds, name, id);
-    int fd = elem ? elem->fd : -1;
+    int fd;
+
+    QEMU_LOCK_GUARD(&cpr_state_lock);
 
+    fd = do_cpr_find_fd(name, id);
     trace_cpr_find_fd(name, id, fd);
     return fd;
 }
 
 void cpr_save_fd(const char *name, int id, int fd)
 {
-    CprFd *elem = find_fd(&cpr_state.fds, name, id);
-    int old_fd = elem ? elem->fd : -1;
+    int old_fd;
+
+    QEMU_LOCK_GUARD(&cpr_state_lock);
 
+    old_fd = do_cpr_find_fd(name, id);
     if (old_fd < 0) {
         do_cpr_save_fd(name, id, fd);
     } else if (old_fd != fd) {
@@ -161,12 +188,17 @@ void cpr_save_fd(const char *name, int id, int fd)
 int cpr_open_fd(const char *path, int flags, const char *name, int id,
                 Error **errp)
 {
-    int fd = cpr_find_fd(name, id);
+    int fd;
+
+    QEMU_LOCK_GUARD(&cpr_state_lock);
+
+    fd = do_cpr_find_fd(name, id);
+    trace_cpr_find_fd(name, id, fd);
 
     if (fd < 0) {
         fd = qemu_open(path, flags, errp);
         if (fd >= 0) {
-            cpr_save_fd(name, id, fd);
+            do_cpr_save_fd(name, id, fd);
         }
     }
     return fd;
@@ -176,6 +208,8 @@ bool cpr_walk_fd(cpr_walk_fd_cb cb)
 {
     CprFd *elem;
 
+    QEMU_LOCK_GUARD(&cpr_state_lock);
+
     QLIST_FOREACH(elem, &cpr_state.fds, next) {
         g_assert(elem->fd >= 0);
         if (!cb(elem->fd)) {
@@ -527,7 +561,9 @@ bool cpr_state_save(MigrationChannel *channel, Error **errp)
     qemu_put_be32(f, QEMU_CPR_FILE_MAGIC);
     qemu_put_be32(f, QEMU_CPR_FILE_VERSION);
 
-    ret = vmstate_save_state(f, &vmstate_cpr_state, &cpr_state, 0, errp);
+    WITH_QEMU_LOCK_GUARD(&cpr_state_lock) {
+        ret = vmstate_save_state(f, &vmstate_cpr_state, &cpr_state, 0, errp);
+    }
     if (ret) {
         qemu_fclose(f);
         return false;
@@ -561,6 +597,8 @@ static void cpr_prune_closed_fds(void)
 {
     CprFd *elem, *next;
 
+    QEMU_LOCK_GUARD(&cpr_state_lock);
+
     QLIST_FOREACH_SAFE(elem, &cpr_state.fds, next, next) {
         if (fcntl(elem->fd, F_GETFD) < 0 && errno == EBADF) {
             trace_cpr_prune_closed_fd(elem->name, elem->id, elem->fd);
@@ -611,7 +649,9 @@ int cpr_state_load(MigrationChannel *channel, Error **errp)
         return -ENOTSUP;
     }
 
-    ret = vmstate_load_state(f, &vmstate_cpr_state, &cpr_state, 1, errp);
+    WITH_QEMU_LOCK_GUARD(&cpr_state_lock) {
+        ret = vmstate_load_state(f, &vmstate_cpr_state, &cpr_state, 1, errp);
+    }
     if (ret) {
         qemu_fclose(f);
         return ret;
-- 
2.53.0

[PATCH hci-8.0 2/2] migration/cpr: lock the ramblocks #VSTOR-145529

[Denis V. Lunev]

From: Denis V. Lunev <den@openvz.org>

The ramblock entries live in the same registry as the descriptors and
are reached the same way, from RAM block setup and teardown, so they
need the same protection.

Take the lock in their accessors too. qemu_ram_free() already holds the
ramlist mutex when it calls in here, and nothing takes the two in the
other order.

Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 migration/cpr.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/migration/cpr.c b/migration/cpr.c
index 78cf430e386..5c0d2c142aa 100644
--- a/migration/cpr.c
+++ b/migration/cpr.c
@@ -244,8 +244,13 @@ static CprRamBlock *find_ramblock(CprRamBlockList *head, const char *name)
 
 void cpr_save_ramblock(const char *name, cpr_ramblock_t ram)
 {
-    CprRamBlock *elem = find_ramblock(&cpr_state.ramblocks, name);
-    cpr_ramblock_t old_ram = elem ? elem->ram : (cpr_ramblock_t) {0, 0};
+    CprRamBlock *elem;
+    cpr_ramblock_t old_ram;
+
+    QEMU_LOCK_GUARD(&cpr_state_lock);
+
+    elem = find_ramblock(&cpr_state.ramblocks, name);
+    old_ram = elem ? elem->ram : (cpr_ramblock_t) {0, 0};
 
     if (old_ram.addr == 0) {
         do_cpr_save_ramblock(name, ram);
@@ -260,7 +265,11 @@ void cpr_save_ramblock(const char *name, cpr_ramblock_t ram)
 
 void cpr_delete_ramblock(const char *name)
 {
-    CprRamBlock *elem = find_ramblock(&cpr_state.ramblocks, name);
+    CprRamBlock *elem;
+
+    QEMU_LOCK_GUARD(&cpr_state_lock);
+
+    elem = find_ramblock(&cpr_state.ramblocks, name);
 
     if (elem) {
         QLIST_REMOVE(elem, next);
@@ -273,8 +282,13 @@ void cpr_delete_ramblock(const char *name)
 
 cpr_ramblock_t cpr_find_ramblock(const char *name)
 {
-    CprRamBlock *elem = find_ramblock(&cpr_state.ramblocks, name);
-    cpr_ramblock_t ram = elem ? elem->ram : (cpr_ramblock_t) {0, 0};
+    CprRamBlock *elem;
+    cpr_ramblock_t ram;
+
+    QEMU_LOCK_GUARD(&cpr_state_lock);
+
+    elem = find_ramblock(&cpr_state.ramblocks, name);
+    ram = elem ? elem->ram : (cpr_ramblock_t) {0, 0};
 
     trace_cpr_find_ramblock(name, (void *) ram.addr, ram.length);
     return ram;
@@ -284,6 +298,8 @@ bool cpr_walk_ramblock(cpr_walk_ramblock_cb cb)
 {
     CprRamBlock *elem;
 
+    QEMU_LOCK_GUARD(&cpr_state_lock);
+
     QLIST_FOREACH(elem, &cpr_state.ramblocks, next) {
         g_assert(elem->ram.addr && elem->ram.length);
         if (!cb(elem->ram, elem->name)) {
-- 
2.53.0

Re: [PATCH hci-8.0 1/2] migration/cpr: lock the saved fds #VSTOR-145529

[Andrey Drobyshev]

  1. Major: lock ordering is created but nowhere documented. This patch
already establishes two orders: ram_list.mutex → cpr_state_lock via
qemu_ram_free() calling cpr_delete_fd() at system/physmem.c:2409, and
chr_write_lock → cpr_state_lock via tcp_chr_disconnect_locked() →
tcp_chr_free_connection() at chardev/char-socket.c:361. The ramlist
order is only mentioned in patch 2, and the chardev order is not
mentioned at all. Put a comment at the lock definition stating it is a
leaf lock and listing what may be held on entry.

  2. Minor: error_report() under the lock inverts the chardev order.
qemu_file_put_fd() calls error_report_err() on failure while
cpr_state_save() holds the lock. From an HMP migrate, error_vprintf()
routes to monitor_vprintf(), which takes the monitor chardev's
chr_write_lock. That is the reverse of order in point 1. No deadlock
today since HMP chardevs live on the main thread, but it is exactly the
kind of remote breakage the commit message argues against. The
error_report() calls in cpr_save_fd() and cpr_save_ramblock() abort
right after, so they are moot. Either note the rule in the comment or
move error reporting out of the guarded region.

  3. Minor: the commit message overstates what stays outside the lock.
vmstate_save_state() and vmstate_load_state() run under the lock and
read or populate vfio_devices and hotplug_cpus. cpr_cpu_hotplug_needed()
fills hotplug_cpus under the lock, while cpr_load_cpu_hotplug_state()
and cpr_free_cpu_hotplug_state() walk and free it unlocked from vl.c.
Both lists are BQL-only in practice. Document per list in
include/migration/cpr.h next to CprState which ones the mutex covers.

  4. Minor: cpr_open_fd() holds the mutex across qemu_open(). A blocking
open of /dev/vfio/N or /dev/iommu stalls every other accessor, including
an iothread chardev disconnect. The fdset path also adds cpr_state_lock
→ mon_fdsets_lock. The atomic find-then-save is a real improvement over
upstream's two separate calls, so keeping it is defensible, but say so
in the commit message and add the ordering to the comment.

  5. Nit: check-then-act across two acquisitions remains. Callers such
as tcp_chr_free_connection() do cpr_find_fd() == fd then
cpr_delete_fd(). The lock protects the list, not the returned fd, and
another thread can replace the entry between the two calls. Harmless for
the existing callers. A cpr_delete_fd_if(name, id, fd) helper would make
the pattern atomic. Optional.

On 9/16/26 5:38 PM, Denis V. Lunev wrote:
> From: Denis V. Lunev <den@openvz.org>
> 
> The cpr state is a process wide registry of the resources that have to
> survive an exec. Entries are added and dropped from realize and teardown
> paths all over the tree, and a caller has no way to tell which thread the
> next caller will run in.
> 
> The state carries no serialization of its own. It is safe today only
> because its callers happen to hold the BQL, and that is a property of
> where those callers live rather than anything the interface promises or
> checks. A caller that is correct within its own subsystem can therefore
> break this one from a distance: two that overlap corrupt the registry,
> and the next lookup walks an entry that has already been freed.
> 
> Give the state a lock of its own instead of relying on that. Entries
> move when a device is realized, when a resource is handed over, and when
> a migration starts, so none of this is on a hot path and an uncontended
> mutex costs nothing.
> 
> Take the lock in every fd accessor and across the save and load walks.
> Holding it over the walk callbacks is safe because none of them comes
> back through this interface. State that is not reached through these
> accessors, such as the VFIO device list that hw/vfio manipulates
> directly, stays outside it.
> 
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> ---
>  migration/cpr.c | 60 ++++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 50 insertions(+), 10 deletions(-)
> 
> diff --git a/migration/cpr.c b/migration/cpr.c
> index f218e6a66d6..78cf430e386 100644
> --- a/migration/cpr.c
> +++ b/migration/cpr.c
> @@ -9,6 +9,7 @@
>  #include "qemu/error-report.h"
>  #include "qapi/error.h"
>  #include "qemu/error-report.h"
> +#include "qemu/lockable.h"
>  #include "hw/vfio/vfio-device.h"
>  #include "migration/cpr.h"
>  #include "migration/misc.h"
> @@ -33,6 +34,14 @@
>  
>  CprState cpr_state;
>  
> +/* Callers are spread across the tree and need not share a thread. */
> +static QemuMutex cpr_state_lock;
> +
> +static void __attribute__((constructor)) cpr_state_lock_init(void)
> +{
> +    qemu_mutex_init(&cpr_state_lock);
> +}
> +
>  /****************************************************************************/
>  
>  typedef struct CprFd {
> @@ -107,10 +116,22 @@ static CprFd *find_fd(CprFdList *head, const char *name, int id)
>      return NULL;
>  }
>  
> -void cpr_delete_fd(const char *name, int id)
> +static int do_cpr_find_fd(const char *name, int id)
>  {
>      CprFd *elem = find_fd(&cpr_state.fds, name, id);
> -    int fd = elem ? elem->fd : -1;
> +
> +    return elem ? elem->fd : -1;
> +}
> +
> +void cpr_delete_fd(const char *name, int id)
> +{
> +    CprFd *elem;
> +    int fd;
> +
> +    QEMU_LOCK_GUARD(&cpr_state_lock);
> +
> +    elem = find_fd(&cpr_state.fds, name, id);
> +    fd = elem ? elem->fd : -1;
>  
>      if (elem) {
>          QLIST_REMOVE(elem, next);
> @@ -125,6 +146,8 @@ void cpr_delete_fd_all(const char *name)
>  {
>      CprFd *elem, *next_elem;
>  
> +    QEMU_LOCK_GUARD(&cpr_state_lock);
> +
>      QLIST_FOREACH_SAFE(elem, &cpr_state.fds, next, next_elem) {
>          if (!strcmp(elem->name, name)) {
>              QLIST_REMOVE(elem, next);
> @@ -136,18 +159,22 @@ void cpr_delete_fd_all(const char *name)
>  
>  int cpr_find_fd(const char *name, int id)
>  {
> -    CprFd *elem = find_fd(&cpr_state.fds, name, id);
> -    int fd = elem ? elem->fd : -1;
> +    int fd;
> +
> +    QEMU_LOCK_GUARD(&cpr_state_lock);
>  
> +    fd = do_cpr_find_fd(name, id);
>      trace_cpr_find_fd(name, id, fd);
>      return fd;
>  }
>  
>  void cpr_save_fd(const char *name, int id, int fd)
>  {
> -    CprFd *elem = find_fd(&cpr_state.fds, name, id);
> -    int old_fd = elem ? elem->fd : -1;
> +    int old_fd;
> +
> +    QEMU_LOCK_GUARD(&cpr_state_lock);
>  
> +    old_fd = do_cpr_find_fd(name, id);
>      if (old_fd < 0) {
>          do_cpr_save_fd(name, id, fd);
>      } else if (old_fd != fd) {
> @@ -161,12 +188,17 @@ void cpr_save_fd(const char *name, int id, int fd)
>  int cpr_open_fd(const char *path, int flags, const char *name, int id,
>                  Error **errp)
>  {
> -    int fd = cpr_find_fd(name, id);
> +    int fd;
> +
> +    QEMU_LOCK_GUARD(&cpr_state_lock);
> +
> +    fd = do_cpr_find_fd(name, id);
> +    trace_cpr_find_fd(name, id, fd);
>  
>      if (fd < 0) {
>          fd = qemu_open(path, flags, errp);
>          if (fd >= 0) {
> -            cpr_save_fd(name, id, fd);
> +            do_cpr_save_fd(name, id, fd);
>          }
>      }
>      return fd;
> @@ -176,6 +208,8 @@ bool cpr_walk_fd(cpr_walk_fd_cb cb)
>  {
>      CprFd *elem;
>  
> +    QEMU_LOCK_GUARD(&cpr_state_lock);
> +
>      QLIST_FOREACH(elem, &cpr_state.fds, next) {
>          g_assert(elem->fd >= 0);
>          if (!cb(elem->fd)) {
> @@ -527,7 +561,9 @@ bool cpr_state_save(MigrationChannel *channel, Error **errp)
>      qemu_put_be32(f, QEMU_CPR_FILE_MAGIC);
>      qemu_put_be32(f, QEMU_CPR_FILE_VERSION);
>  
> -    ret = vmstate_save_state(f, &vmstate_cpr_state, &cpr_state, 0, errp);
> +    WITH_QEMU_LOCK_GUARD(&cpr_state_lock) {
> +        ret = vmstate_save_state(f, &vmstate_cpr_state, &cpr_state, 0, errp);
> +    }
>      if (ret) {
>          qemu_fclose(f);
>          return false;
> @@ -561,6 +597,8 @@ static void cpr_prune_closed_fds(void)
>  {
>      CprFd *elem, *next;
>  
> +    QEMU_LOCK_GUARD(&cpr_state_lock);
> +
>      QLIST_FOREACH_SAFE(elem, &cpr_state.fds, next, next) {
>          if (fcntl(elem->fd, F_GETFD) < 0 && errno == EBADF) {
>              trace_cpr_prune_closed_fd(elem->name, elem->id, elem->fd);
> @@ -611,7 +649,9 @@ int cpr_state_load(MigrationChannel *channel, Error **errp)
>          return -ENOTSUP;
>      }
>  
> -    ret = vmstate_load_state(f, &vmstate_cpr_state, &cpr_state, 1, errp);
> +    WITH_QEMU_LOCK_GUARD(&cpr_state_lock) {
> +        ret = vmstate_load_state(f, &vmstate_cpr_state, &cpr_state, 1, errp);
> +    }
>      if (ret) {
>          qemu_fclose(f);
>          return ret;

Re: [PATCH hci-8.0 2/2] migration/cpr: lock the ramblocks #VSTOR-145529

[Andrey Drobyshev]

1. Minor: the ordering claim belongs in code, and in patch 1. "Nothing
takes the two in the other order" checks out: the walk callbacks in
cpr-exec only do madvise and mprotect, and the ramblock vmstate has no
callbacks. But the order was already forced by patch 1, so fold the
statement into the lock comment there.

  2. Nit: ram_block_add() does find, then save or delete, in three
separate acquisitions. It is correct only because the ramlist mutex is
held across all three at system/physmem.c:1913-2058. A one-line comment
saying the ramlist mutex is the real serializer there would save the
next reader the same analysis.

On 9/16/26 5:38 PM, Denis V. Lunev wrote:
> From: Denis V. Lunev <den@openvz.org>
> 
> The ramblock entries live in the same registry as the descriptors and
> are reached the same way, from RAM block setup and teardown, so they
> need the same protection.
> 
> Take the lock in their accessors too. qemu_ram_free() already holds the
> ramlist mutex when it calls in here, and nothing takes the two in the
> other order.
> 
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> ---
>  migration/cpr.c | 26 +++++++++++++++++++++-----
>  1 file changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/migration/cpr.c b/migration/cpr.c
> index 78cf430e386..5c0d2c142aa 100644
> --- a/migration/cpr.c
> +++ b/migration/cpr.c
> @@ -244,8 +244,13 @@ static CprRamBlock *find_ramblock(CprRamBlockList *head, const char *name)
>  
>  void cpr_save_ramblock(const char *name, cpr_ramblock_t ram)
>  {
> -    CprRamBlock *elem = find_ramblock(&cpr_state.ramblocks, name);
> -    cpr_ramblock_t old_ram = elem ? elem->ram : (cpr_ramblock_t) {0, 0};
> +    CprRamBlock *elem;
> +    cpr_ramblock_t old_ram;
> +
> +    QEMU_LOCK_GUARD(&cpr_state_lock);
> +
> +    elem = find_ramblock(&cpr_state.ramblocks, name);
> +    old_ram = elem ? elem->ram : (cpr_ramblock_t) {0, 0};
>  
>      if (old_ram.addr == 0) {
>          do_cpr_save_ramblock(name, ram);
> @@ -260,7 +265,11 @@ void cpr_save_ramblock(const char *name, cpr_ramblock_t ram)
>  
>  void cpr_delete_ramblock(const char *name)
>  {
> -    CprRamBlock *elem = find_ramblock(&cpr_state.ramblocks, name);
> +    CprRamBlock *elem;
> +
> +    QEMU_LOCK_GUARD(&cpr_state_lock);
> +
> +    elem = find_ramblock(&cpr_state.ramblocks, name);
>  
>      if (elem) {
>          QLIST_REMOVE(elem, next);
> @@ -273,8 +282,13 @@ void cpr_delete_ramblock(const char *name)
>  
>  cpr_ramblock_t cpr_find_ramblock(const char *name)
>  {
> -    CprRamBlock *elem = find_ramblock(&cpr_state.ramblocks, name);
> -    cpr_ramblock_t ram = elem ? elem->ram : (cpr_ramblock_t) {0, 0};
> +    CprRamBlock *elem;
> +    cpr_ramblock_t ram;
> +
> +    QEMU_LOCK_GUARD(&cpr_state_lock);
> +
> +    elem = find_ramblock(&cpr_state.ramblocks, name);
> +    ram = elem ? elem->ram : (cpr_ramblock_t) {0, 0};
>  
>      trace_cpr_find_ramblock(name, (void *) ram.addr, ram.length);
>      return ram;
> @@ -284,6 +298,8 @@ bool cpr_walk_ramblock(cpr_walk_ramblock_cb cb)
>  {
>      CprRamBlock *elem;
>  
> +    QEMU_LOCK_GUARD(&cpr_state_lock);
> +
>      QLIST_FOREACH(elem, &cpr_state.ramblocks, next) {
>          g_assert(elem->ram.addr && elem->ram.length);
>          if (!cb(elem->ram, elem->name)) {