[PATCH 0/2] libvhost-user: fix postcopy shared memory faults

[PATCH 0/2] libvhost-user: fix postcopy shared memory faults

[Bin Guo]

These two independent bugs both sit on the libvhost-user postcopy path and
together make postcopy migration of a guest with a libvhost-user backend
fail.  They are only reachable when the backend negotiates
VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS, i.e. it adds memory regions one
by one with VHOST_USER_ADD_MEM_REG instead of VHOST_USER_SET_MEM_TABLE.

Patch 1 stops the backend from panicking on the "all postcopy client bases
received" ack, which QEMU sends as an fd-less VHOST_USER_ADD_MEM_REG.

Patch 2 makes the backend return its mapping address for a region added
that way, without which QEMU cannot translate the backend's fault
addresses back to a RAMBlock.

Reproduced and fixed with contrib/vhost-user-bridge as the net backend of
a postcopy migrated guest: before the series the destination backend
either panics or QEMU reports "Failed to find region for fault"; after it
the migration completes.

Bin Guo (2):
  libvhost-user: accept the postcopy client base ack in vu_add_mem_reg()
  libvhost-user: return the backend mapping address for added regions

 subprojects/libvhost-user/libvhost-user.c | 34 ++++++++++++++---------
 1 file changed, 21 insertions(+), 13 deletions(-)

-- 
2.50.1 (Apple Git-155)

[PATCH 1/2] libvhost-user: accept the postcopy client base ack in vu_add_mem_reg()

[Bin Guo]

In postcopy mode QEMU signals that it has collected all the postcopy client
bases by sending a VHOST_USER_ADD_MEM_REG message with a u64 payload of 0
and no file descriptor (see vhost_user_add_remove_regions()).
vu_add_mem_reg() has a case for that message, but only reaches it after
validating the fd count of a regular region, so the ack is rejected first:

  VHOST_USER_ADD_MEM_REG received 0 fds - only 1 fd should be sent for
  this message type

This kills the backend during memory table setup, making postcopy unusable
for any libvhost-user backend that negotiates
VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS.

Recognise the ack before validating the fd count.

Fixes: 9f4e63491b ("libvhost-user: Add vu_add_mem_reg input validation")
Signed-off-by: Bin Guo <guobin@linux.alibaba.com>
---
 subprojects/libvhost-user/libvhost-user.c | 26 ++++++++++++-----------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c
index a74d814bb4..248550aae1 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -948,6 +948,20 @@ static bool
 vu_add_mem_reg(VuDev *dev, VhostUserMsg *vmsg) {
     VhostUserMemoryRegion m = vmsg->payload.memreg.region, *msg_region = &m;
 
+    /*
+     * If we are in postcopy mode and we receive a u64 payload with a 0 value
+     * we know all the postcopy client bases have been received, and we
+     * should start generating faults.  This message carries no file
+     * descriptor, so it has to be recognised before the fd count of a real
+     * region is validated below.
+     */
+    if (dev->postcopy_listening &&
+        vmsg->size == sizeof(vmsg->payload.u64) &&
+        vmsg->payload.u64 == 0) {
+        (void)generate_faults(dev);
+        return false;
+    }
+
     if (vmsg->fd_num != 1) {
         vmsg_close_fds(vmsg);
         vu_panic(dev, "VHOST_USER_ADD_MEM_REG received %d fds - only 1 fd "
@@ -971,18 +985,6 @@ vu_add_mem_reg(VuDev *dev, VhostUserMsg *vmsg) {
         return false;
     }
 
-    /*
-     * If we are in postcopy mode and we receive a u64 payload with a 0 value
-     * we know all the postcopy client bases have been received, and we
-     * should start generating faults.
-     */
-    if (dev->postcopy_listening &&
-        vmsg->size == sizeof(vmsg->payload.u64) &&
-        vmsg->payload.u64 == 0) {
-        (void)generate_faults(dev);
-        return false;
-    }
-
     _vu_add_mem_reg(dev, msg_region, vmsg->fds[0]);
     close(vmsg->fds[0]);
 
-- 
2.50.1 (Apple Git-155)

[PATCH 2/2] libvhost-user: return the backend mapping address for added regions

[Bin Guo]

In postcopy mode the backend must tell QEMU where it mapped a region, so
that QEMU can translate the backend's fault addresses back into a RAMBlock
and offset.  vu_add_mem_reg() works on a copy of the region and lets
_vu_add_mem_reg() fill the mapping address into that copy, but never writes
it back into the reply payload.  QEMU then stores its own address as the
postcopy client base, so fault resolution fails:

  vhost_user_postcopy_fault_handler: Failed to find region for fault ...

The VHOST_USER_SET_MEM_TABLE path is unaffected, it fills the payload
directly.  Put the updated region back into the payload before replying; a
pointer into the payload cannot be used instead, as VhostUserMsg is packed.

Fixes: ec94c8e621 ("Support adding individual regions in libvhost-user")
Signed-off-by: Bin Guo <guobin@linux.alibaba.com>
---
 subprojects/libvhost-user/libvhost-user.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c
index 248550aae1..2fed792e85 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -989,7 +989,13 @@ vu_add_mem_reg(VuDev *dev, VhostUserMsg *vmsg) {
     close(vmsg->fds[0]);
 
     if (dev->postcopy_listening) {
-        /* Send the message back to qemu with the addresses filled in. */
+        /*
+         * Send the message back to qemu with the addresses filled in.
+         * _vu_add_mem_reg() worked on our copy of the region, so it has to be
+         * put back into the message payload.  A pointer into the payload
+         * cannot be handed out instead, VhostUserMsg is packed.
+         */
+        vmsg->payload.memreg.region = m;
         vmsg->fd_num = 0;
         DPRINT("Successfully added new region in postcopy\n");
         return true;
-- 
2.50.1 (Apple Git-155)