diff mbox series

[RFC,v4,11/49] multi-process: setup memory manager for remote device

Message ID 5bfab4fa2f7f12137d0030e08a494d9ac3e11f04.1571905346.git.jag.raman@oracle.com
State New
Headers show
Series Initial support of multi-process qemu | expand

Commit Message

Jag Raman Oct. 24, 2019, 9:08 a.m. UTC 
sync_sysmem_msg_t message format is defined. It is used to send
file descriptors of the RAM regions to remote device.
RAM on the remote device is configured with a set of file descriptors.
Old RAM regions are deleted and new regions, each with an fd, is
added to the RAM.

Signed-off-by: Jagannathan Raman <jag.raman@oracle.com>
Signed-off-by: John G Johnson <john.g.johnson@oracle.com>
Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com>
---
 Makefile.target          |  2 +
 include/io/mpqemu-link.h | 11 ++++++
 include/remote/memory.h  | 34 +++++++++++++++++
 remote/memory.c          | 99 ++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 146 insertions(+)
 create mode 100644 include/remote/memory.h
 create mode 100644 remote/memory.c

Comments

Stefan Hajnoczi Nov. 13, 2019, 4:33 p.m. UTC | #1 
On Thu, Oct 24, 2019 at 05:08:52AM -0400, Jagannathan Raman wrote:
> +static void remote_ram_destructor(MemoryRegion *mr)
> +{
> +    qemu_ram_free(mr->ram_block);
> +}
> +
> +static void remote_ram_init_from_fd(MemoryRegion *mr, int fd, uint64_t size,
> +                                    ram_addr_t offset, Error **errp)
> +{
> +    char *name = g_strdup_printf("%d", fd);
> +
> +    memory_region_init(mr, NULL, name, size);
> +    mr->ram = true;
> +    mr->terminates = true;
> +    mr->destructor = NULL;
> +    mr->align = 0;
> +    mr->ram_block = qemu_ram_alloc_from_fd(size, mr, RAM_SHARED, fd, offset,
> +                                           errp);
> +    mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
> +
> +    g_free(name);
> +}

This is not specific to remote/memory.c and could be shared in case
something else in QEMU wants to initialize from an fd.

> +
> +void remote_sysmem_reconfig(MPQemuMsg *msg, Error **errp)
> +{
> +    sync_sysmem_msg_t *sysmem_info = &msg->data1.sync_sysmem;

A possible security issue with MPQemuMsg: was the message size
validatedb before we access msg->data1.sync_sysmem?

If not, then we might access uninitialized data.  I didn't see if there
is a single place in the code that always zeroes msg, but I think the
answer is no.  Accessing uninitialized data could expose the old
contents of the stack/heap to the other process.  Information leaks like
this can be used to defeat address-space randomization because the other
process may learn about our memory layout if there are memory addresses
in the uninitialized data.
Jag Raman Nov. 13, 2019, 4:34 p.m. UTC | #2 
On 11/13/2019 11:33 AM, Stefan Hajnoczi wrote:
> On Thu, Oct 24, 2019 at 05:08:52AM -0400, Jagannathan Raman wrote:
>> +static void remote_ram_destructor(MemoryRegion *mr)
>> +{
>> +    qemu_ram_free(mr->ram_block);
>> +}
>> +
>> +static void remote_ram_init_from_fd(MemoryRegion *mr, int fd, uint64_t size,
>> +                                    ram_addr_t offset, Error **errp)
>> +{
>> +    char *name = g_strdup_printf("%d", fd);
>> +
>> +    memory_region_init(mr, NULL, name, size);
>> +    mr->ram = true;
>> +    mr->terminates = true;
>> +    mr->destructor = NULL;
>> +    mr->align = 0;
>> +    mr->ram_block = qemu_ram_alloc_from_fd(size, mr, RAM_SHARED, fd, offset,
>> +                                           errp);
>> +    mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
>> +
>> +    g_free(name);
>> +}
> 
> This is not specific to remote/memory.c and could be shared in case
> something else in QEMU wants to initialize from an fd.
> 
>> +
>> +void remote_sysmem_reconfig(MPQemuMsg *msg, Error **errp)
>> +{
>> +    sync_sysmem_msg_t *sysmem_info = &msg->data1.sync_sysmem;
> 
> A possible security issue with MPQemuMsg: was the message size
> validatedb before we access msg->data1.sync_sysmem?
> 
> If not, then we might access uninitialized data.  I didn't see if there
> is a single place in the code that always zeroes msg, but I think the
> answer is no.  Accessing uninitialized data could expose the old
> contents of the stack/heap to the other process.  Information leaks like
> this can be used to defeat address-space randomization because the other
> process may learn about our memory layout if there are memory addresses
> in the uninitialized data.

Thanks for the feedback. Will do.

--
Jag

>
diff mbox series

Patch

diff --git a/Makefile.target b/Makefile.target
index e454aae..547f10e 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -136,6 +136,8 @@  remote-pci-tgt-obj-$(CONFIG_MPQEMU) += stubs/xen-mapcache.o
 remote-pci-tgt-obj-$(CONFIG_MPQEMU) += stubs/audio.o
 remote-pci-tgt-obj-$(CONFIG_MPQEMU) += stubs/monitor.o
 
+remote-pci-tgt-obj-$(CONFIG_MPQEMU) += remote/memory.o
+
 #########################################################
 # Linux user emulator target
 
diff --git a/include/io/mpqemu-link.h b/include/io/mpqemu-link.h
index dee2dd3..7ef8207 100644
--- a/include/io/mpqemu-link.h
+++ b/include/io/mpqemu-link.h
@@ -36,6 +36,8 @@ 
 
 #include "qom/object.h"
 #include "qemu/thread.h"
+#include "exec/cpu-common.h"
+#include "exec/hwaddr.h"
 
 #define TYPE_MPQEMU_LINK "mpqemu-link"
 #define MPQEMU_LINK(obj) \
@@ -49,6 +51,7 @@ 
  * mpqemu_cmd_t:
  * CONF_READ        PCI config. space read
  * CONF_WRITE       PCI config. space write
+ * SYNC_SYSMEM      Shares QEMU's RAM with remote device's RAM
  *
  * proc_cmd_t enum type to specify the command to be executed on the remote
  * device.
@@ -57,6 +60,7 @@  typedef enum {
     INIT = 0,
     CONF_READ,
     CONF_WRITE,
+    SYNC_SYSMEM,
     MAX,
 } mpqemu_cmd_t;
 
@@ -74,12 +78,19 @@  typedef enum {
  *
  */
 typedef struct {
+    hwaddr gpas[REMOTE_MAX_FDS];
+    uint64_t sizes[REMOTE_MAX_FDS];
+    ram_addr_t offsets[REMOTE_MAX_FDS];
+} sync_sysmem_msg_t;
+
+typedef struct {
     mpqemu_cmd_t cmd;
     int bytestream;
     size_t size;
 
     union {
         uint64_t u64;
+        sync_sysmem_msg_t sync_sysmem;
     } data1;
 
     int fds[REMOTE_MAX_FDS];
diff --git a/include/remote/memory.h b/include/remote/memory.h
new file mode 100644
index 0000000..0bb637f
--- /dev/null
+++ b/include/remote/memory.h
@@ -0,0 +1,34 @@ 
+/*
+ * Memory manager for remote device
+ *
+ * Copyright 2019, Oracle and/or its affiliates.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef REMOTE_MEMORY_H
+#define REMOTE_MEMORY_H
+
+#include "qemu/osdep.h"
+#include "exec/hwaddr.h"
+#include "io/mpqemu-link.h"
+
+void remote_sysmem_reconfig(MPQemuMsg *msg, Error **errp);
+
+#endif
diff --git a/remote/memory.c b/remote/memory.c
new file mode 100644
index 0000000..a70027e
--- /dev/null
+++ b/remote/memory.c
@@ -0,0 +1,99 @@ 
+/*
+ * Memory manager for remote device
+ *
+ * Copyright 2019, Oracle and/or its affiliates.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include <stdint.h>
+#include <sys/types.h>
+
+#include "qemu/osdep.h"
+#include "qemu/queue.h"
+#include "qemu-common.h"
+#include "remote/memory.h"
+#include "exec/memory.h"
+#include "exec/address-spaces.h"
+#include "cpu.h"
+#include "exec/ram_addr.h"
+#include "io/mpqemu-link.h"
+#include "qemu/main-loop.h"
+#include "qapi/error.h"
+
+static void remote_ram_destructor(MemoryRegion *mr)
+{
+    qemu_ram_free(mr->ram_block);
+}
+
+static void remote_ram_init_from_fd(MemoryRegion *mr, int fd, uint64_t size,
+                                    ram_addr_t offset, Error **errp)
+{
+    char *name = g_strdup_printf("%d", fd);
+
+    memory_region_init(mr, NULL, name, size);
+    mr->ram = true;
+    mr->terminates = true;
+    mr->destructor = NULL;
+    mr->align = 0;
+    mr->ram_block = qemu_ram_alloc_from_fd(size, mr, RAM_SHARED, fd, offset,
+                                           errp);
+    mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
+
+    g_free(name);
+}
+
+void remote_sysmem_reconfig(MPQemuMsg *msg, Error **errp)
+{
+    sync_sysmem_msg_t *sysmem_info = &msg->data1.sync_sysmem;
+    MemoryRegion *sysmem, *subregion, *next;
+    Error *local_err = NULL;
+    int region;
+
+    sysmem = get_system_memory();
+
+    qemu_mutex_lock_iothread();
+
+    memory_region_transaction_begin();
+
+    QTAILQ_FOREACH_SAFE(subregion, &sysmem->subregions, subregions_link, next) {
+        if (subregion->ram) {
+            memory_region_del_subregion(sysmem, subregion);
+            remote_ram_destructor(subregion);
+        }
+    }
+
+    for (region = 0; region < msg->num_fds; region++) {
+        subregion = g_new(MemoryRegion, 1);
+        remote_ram_init_from_fd(subregion, msg->fds[region],
+                                sysmem_info->sizes[region],
+                                sysmem_info->offsets[region], &local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
+            break;
+        }
+
+        memory_region_add_subregion(sysmem, sysmem_info->gpas[region],
+                                    subregion);
+    }
+
+    memory_region_transaction_commit();
+
+    qemu_mutex_unlock_iothread();
+}