[PULL 00/41] Misc patches for 2017-06-15

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
43 messages Options
123
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PULL 00/41] Misc patches for 2017-06-15

Paolo Bonzini-5
The following changes since commit 3f0602927b120a480b35dcf58cf6f95435b3ae91:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20170613' into staging (2017-06-13 15:49:07 +0100)

are available in the git repository at:


  git://github.com/bonzini/qemu.git tags/for-upstream

for you to fetch changes up to 49cc0340f8be18871319ffec6efc72147e73ff0b:

  vhost-user-scsi: Introduce a vhost-user-scsi sample application (2017-06-15 11:18:40 +0200)

----------------------------------------------------------------
* nbd and qemu-nbd fixes (Eric, Max)
* nbd refactoring (Vladimir)
* vhost-user-scsi, take N+1 (Felipe)
* replace memory_region_set_fd with memory_region_init_ram_from_fd (Marc-André)
* docs/ movement (Paolo)
* megasas TOCTOU fixes (Paolo)
* make async_safe_run_on_cpu work on kvm/hax accelerators (Paolo)
* Build system and poison.h improvements (Thomas)
* -accel thread=xxx fix (Thomas)
* move files to accel/ (Yang Zhong)

----------------------------------------------------------------
Eric Blake (1):
      nbd: Fix regression on resiliency to port scan

Felipe Franciosi (2):
      vhost-user-scsi: Introduce vhost-user-scsi host device
      vhost-user-scsi: Introduce a vhost-user-scsi sample application

Marc-André Lureau (6):
      exec: check kvm mmu notifiers earlier
      exec: split file_ram_alloc()
      exec: split qemu_ram_alloc_from_file()
      Add memory_region_init_ram_from_fd()
      ivshmem: use ram_from_fd()
      memory: remove memory_region_set_fd

Max Reitz (1):
      qemu-nbd: Ignore SIGPIPE

Paolo Bonzini (10):
      megasas: add qtest
      megasas: do not read sense length more than once from frame
      megasas: do not read iovec count more than once from frame
      megasas: do not read DCMD opcode more than once from frame
      megasas: do not read command more than once from frame
      megasas: do not read SCSI req parameters more than once from frame
      megasas: always store SCSIRequest* into MegasasCmd
      hax-all: make async_safe_run_on_cpu safe on HAX too
      docs: create interop/ subdirectory
      qemu-doc: include version number

Roman Kagan (1):
      kvm-all: make async_safe_run_on_cpu safe on kvm too

Thomas Huth (4):
      Makefile: Do not generate files if "configure" has not been run yet
      vl: Fix broken thread=xxx option of the --accel parameter
      include/exec/poison: Add missing TARGET defines
      include/exec/poison: Mark some CONFIG defines as poisoned, too

Vladimir Sementsov-Ogievskiy (12):
      nbd: rename read_sync and friends
      nbd: make nbd_drop public
      nbd/server: get rid of nbd_negotiate_read and friends
      nbd/server: get rid of ssize_t
      nbd/server: refactor nbd_co_send_reply
      nbd/server: get rid of EAGAIN dead code
      nbd/server: refactor nbd_co_receive_request
      nbd/server: remove NBDClientNewData
      nbd/server: nbd_negotiate: fix error path
      nbd/server: get rid of fail: return rc
      nbd/server: rename rc to ret
      nbd/server: refactor nbd_trip

Yang Zhong (4):
      accel: split the tcg accelerator from accel.c file
      tcg: move tcg related files into accel/tcg/ subdirectory
      tcg: move tcg backend files into accel/tcg/
      accel: move kvm related accelerator files into accel/

 .gitignore                                         |  17 +-
 Makefile                                           |  77 +-
 Makefile.objs                                      |   7 +-
 Makefile.target                                    |  12 +-
 accel/Makefile.objs                                |   4 +
 accel.c => accel/accel.c                           |  27 -
 accel/kvm/Makefile.objs                            |   1 +
 kvm-all.c => accel/kvm/kvm-all.c                   |   4 +-
 accel/kvm/trace-events                             |  15 +
 accel/stubs/Makefile.objs                          |   1 +
 kvm-stub.c => accel/stubs/kvm-stub.c               |   0
 accel/tcg/Makefile.objs                            |   3 +
 cpu-exec-common.c => accel/tcg/cpu-exec-common.c   |   0
 cpu-exec.c => accel/tcg/cpu-exec.c                 |   2 +-
 cputlb.c => accel/tcg/cputlb.c                     |   0
 accel/tcg/tcg-all.c                                |  61 ++
 accel/tcg/trace-events                             |  10 +
 translate-all.c => accel/tcg/translate-all.c       |   2 +-
 translate-all.h => accel/tcg/translate-all.h       |   0
 translate-common.c => accel/tcg/translate-common.c |   0
 block/nbd-client.c                                 |   8 +-
 blockdev-nbd.c                                     |   6 +-
 configure                                          |   4 +-
 contrib/libvhost-user/libvhost-user.h              |  11 +-
 contrib/vhost-user-scsi/Makefile.objs              |   1 +
 contrib/vhost-user-scsi/vhost-user-scsi.c          | 886 +++++++++++++++++++++
 default-configs/pci.mak                            |   1 +
 default-configs/s390x-softmmu.mak                  |   1 +
 docs/{specs => interop}/parallels.txt              |   0
 docs/{specs => interop}/qcow2.txt                  |   0
 docs/{specs => interop}/qed_spec.txt               |   0
 docs/{ => interop}/qemu-ga-ref.texi                |   0
 docs/{ => interop}/qemu-qmp-ref.texi               |   0
 docs/{ => interop}/qmp-intro.txt                   |   0
 docs/{ => interop}/qmp-spec.txt                    |   0
 docs/{specs => interop}/vhost-user.txt             |   0
 .../{ => interop}/vnc-ledstate-Pseudo-encoding.txt |   0
 exec.c                                             | 116 +--
 hw/misc/ivshmem.c                                  |  14 +-
 hw/scsi/Makefile.objs                              |   1 +
 hw/scsi/megasas.c                                  | 175 ++--
 hw/scsi/vhost-scsi-common.c                        |   1 -
 hw/scsi/vhost-user-scsi.c                          | 205 +++++
 hw/virtio/virtio-pci.c                             |  58 ++
 hw/virtio/virtio-pci.h                             |  11 +
 include/block/nbd.h                                |  10 +-
 include/exec/memory.h                              |  31 +-
 include/exec/poison.h                              |  31 +
 include/exec/ram_addr.h                            |   3 +
 include/hw/virtio/vhost-user-scsi.h                |  35 +
 include/hw/virtio/virtio-scsi.h                    |   2 +
 memory.c                                           |  26 +-
 nbd/client.c                                       |  64 +-
 nbd/common.c                                       |  34 +-
 nbd/nbd-internal.h                                 |  28 +-
 nbd/server.c                                       | 343 +++-----
 qemu-doc.texi                                      |   5 +-
 qemu-nbd.c                                         |   8 +-
 rules.mak                                          |   2 +-
 target/i386/hax-all.c                              |   3 +-
 tcg-runtime.c => tcg/tcg-runtime.c                 |   0
 tci.c => tcg/tci.c                                 |   0
 tests/Makefile.include                             |   3 +
 tests/megasas-test.c                               |  86 ++
 trace-events                                       |  22 -
 vl.c                                               |  13 +-
 66 files changed, 1895 insertions(+), 596 deletions(-)
 create mode 100644 accel/Makefile.objs
 rename accel.c => accel/accel.c (87%)
 create mode 100644 accel/kvm/Makefile.objs
 rename kvm-all.c => accel/kvm/kvm-all.c (99%)
 create mode 100644 accel/kvm/trace-events
 create mode 100644 accel/stubs/Makefile.objs
 rename kvm-stub.c => accel/stubs/kvm-stub.c (100%)
 create mode 100644 accel/tcg/Makefile.objs
 rename cpu-exec-common.c => accel/tcg/cpu-exec-common.c (100%)
 rename cpu-exec.c => accel/tcg/cpu-exec.c (99%)
 rename cputlb.c => accel/tcg/cputlb.c (100%)
 create mode 100644 accel/tcg/tcg-all.c
 create mode 100644 accel/tcg/trace-events
 rename translate-all.c => accel/tcg/translate-all.c (99%)
 rename translate-all.h => accel/tcg/translate-all.h (100%)
 rename translate-common.c => accel/tcg/translate-common.c (100%)
 create mode 100644 contrib/vhost-user-scsi/Makefile.objs
 create mode 100644 contrib/vhost-user-scsi/vhost-user-scsi.c
 rename docs/{specs => interop}/parallels.txt (100%)
 rename docs/{specs => interop}/qcow2.txt (100%)
 rename docs/{specs => interop}/qed_spec.txt (100%)
 rename docs/{ => interop}/qemu-ga-ref.texi (100%)
 rename docs/{ => interop}/qemu-qmp-ref.texi (100%)
 rename docs/{ => interop}/qmp-intro.txt (100%)
 rename docs/{ => interop}/qmp-spec.txt (100%)
 rename docs/{specs => interop}/vhost-user.txt (100%)
 rename docs/{ => interop}/vnc-ledstate-Pseudo-encoding.txt (100%)
 create mode 100644 hw/scsi/vhost-user-scsi.c
 create mode 100644 include/hw/virtio/vhost-user-scsi.h
 rename tcg-runtime.c => tcg/tcg-runtime.c (100%)
 rename tci.c => tcg/tci.c (100%)
 create mode 100644 tests/megasas-test.c
--
1.8.3.1


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PULL 01/41] exec: check kvm mmu notifiers earlier

Paolo Bonzini-5
From: Marc-André Lureau <[hidden email]>

Move kvm mmu notifiers check before calling file_ram_alloc(), with the
other xen precondition. (file_ram_alloc() will be reused in other cases
than -mem-path).

Signed-off-by: Marc-André Lureau <[hidden email]>
Message-Id: <[hidden email]>
Signed-off-by: Paolo Bonzini <[hidden email]>
---
 exec.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/exec.c b/exec.c
index a93e209..a5be851 100644
--- a/exec.c
+++ b/exec.c
@@ -1495,12 +1495,6 @@ static void *file_ram_alloc(RAMBlock *block,
     int fd = -1;
     int64_t file_size;
 
-    if (kvm_enabled() && !kvm_has_sync_mmu()) {
-        error_setg(errp,
-                   "host lacks kvm mmu notifiers, -mem-path unsupported");
-        return NULL;
-    }
-
     for (;;) {
         fd = open(path, O_RDWR);
         if (fd >= 0) {
@@ -1943,6 +1937,12 @@ RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
         return NULL;
     }
 
+    if (kvm_enabled() && !kvm_has_sync_mmu()) {
+        error_setg(errp,
+                   "host lacks kvm mmu notifiers, -mem-path unsupported");
+        return NULL;
+    }
+
     if (phys_mem_alloc != qemu_anon_ram_alloc) {
         /*
          * file_ram_alloc() needs to allocate just like
--
1.8.3.1



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PULL 02/41] exec: split file_ram_alloc()

Paolo Bonzini-5
In reply to this post by Paolo Bonzini-5
From: Marc-André Lureau <[hidden email]>

Move file opening part in a seperate function, file_ram_open(). This
allows for reuse of file_ram_alloc() with a given fd.

Signed-off-by: Marc-André Lureau <[hidden email]>
Message-Id: <[hidden email]>
Signed-off-by: Paolo Bonzini <[hidden email]>
---
 exec.c | 83 +++++++++++++++++++++++++++++++++++++-----------------------------
 1 file changed, 46 insertions(+), 37 deletions(-)

diff --git a/exec.c b/exec.c
index a5be851..42a5a7c 100644
--- a/exec.c
+++ b/exec.c
@@ -1482,19 +1482,17 @@ static int64_t get_file_size(int fd)
     return size;
 }
 
-static void *file_ram_alloc(RAMBlock *block,
-                            ram_addr_t memory,
-                            const char *path,
-                            Error **errp)
+static int file_ram_open(const char *path,
+                         const char *region_name,
+                         bool *created,
+                         Error **errp)
 {
-    bool unlink_on_error = false;
     char *filename;
     char *sanitized_name;
     char *c;
-    void *area = MAP_FAILED;
     int fd = -1;
-    int64_t file_size;
 
+    *created = false;
     for (;;) {
         fd = open(path, O_RDWR);
         if (fd >= 0) {
@@ -1505,13 +1503,13 @@ static void *file_ram_alloc(RAMBlock *block,
             /* @path names a file that doesn't exist, create it */
             fd = open(path, O_RDWR | O_CREAT | O_EXCL, 0644);
             if (fd >= 0) {
-                unlink_on_error = true;
+                *created = true;
                 break;
             }
         } else if (errno == EISDIR) {
             /* @path names a directory, create a file there */
             /* Make name safe to use with mkstemp by replacing '/' with '_'. */
-            sanitized_name = g_strdup(memory_region_name(block->mr));
+            sanitized_name = g_strdup(region_name);
             for (c = sanitized_name; *c != '\0'; c++) {
                 if (*c == '/') {
                     *c = '_';
@@ -1534,7 +1532,7 @@ static void *file_ram_alloc(RAMBlock *block,
             error_setg_errno(errp, errno,
                              "can't open backing store %s for guest RAM",
                              path);
-            goto error;
+            return -1;
         }
         /*
          * Try again on EINTR and EEXIST.  The latter happens when
@@ -1542,6 +1540,17 @@ static void *file_ram_alloc(RAMBlock *block,
          */
     }
 
+    return fd;
+}
+
+static void *file_ram_alloc(RAMBlock *block,
+                            ram_addr_t memory,
+                            int fd,
+                            bool truncate,
+                            Error **errp)
+{
+    void *area;
+
     block->page_size = qemu_fd_getpagesize(fd);
     block->mr->align = block->page_size;
 #if defined(__s390x__)
@@ -1550,20 +1559,11 @@ static void *file_ram_alloc(RAMBlock *block,
     }
 #endif
 
-    file_size = get_file_size(fd);
-
     if (memory < block->page_size) {
         error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal to "
                    "or larger than page size 0x%zx",
                    memory, block->page_size);
-        goto error;
-    }
-
-    if (file_size > 0 && file_size < memory) {
-        error_setg(errp, "backing store %s size 0x%" PRIx64
-                   " does not match 'size' option 0x" RAM_ADDR_FMT,
-                   path, file_size, memory);
-        goto error;
+        return NULL;
     }
 
     memory = ROUND_UP(memory, block->page_size);
@@ -1582,7 +1582,7 @@ static void *file_ram_alloc(RAMBlock *block,
      * those labels. Therefore, extending the non-empty backend file
      * is disabled as well.
      */
-    if (!file_size && ftruncate(fd, memory)) {
+    if (truncate && ftruncate(fd, memory)) {
         perror("ftruncate");
     }
 
@@ -1591,30 +1591,19 @@ static void *file_ram_alloc(RAMBlock *block,
     if (area == MAP_FAILED) {
         error_setg_errno(errp, errno,
                          "unable to map backing store for guest RAM");
-        goto error;
+        return NULL;
     }
 
     if (mem_prealloc) {
         os_mem_prealloc(fd, area, memory, smp_cpus, errp);
         if (errp && *errp) {
-            goto error;
+            qemu_ram_munmap(area, memory);
+            return NULL;
         }
     }
 
     block->fd = fd;
     return area;
-
-error:
-    if (area != MAP_FAILED) {
-        qemu_ram_munmap(area, memory);
-    }
-    if (unlink_on_error) {
-        unlink(path);
-    }
-    if (fd != -1) {
-        close(fd);
-    }
-    return NULL;
 }
 #endif
 
@@ -1931,6 +1920,9 @@ RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
 {
     RAMBlock *new_block;
     Error *local_err = NULL;
+    int fd;
+    bool created;
+    int64_t file_size;
 
     if (xen_enabled()) {
         error_setg(errp, "-mem-path not supported with Xen");
@@ -1954,15 +1946,32 @@ RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
         return NULL;
     }
 
+    fd = file_ram_open(mem_path, memory_region_name(mr), &created, errp);
+    if (fd < 0) {
+        return NULL;
+    }
+
     size = HOST_PAGE_ALIGN(size);
+    file_size = get_file_size(fd);
+    if (file_size > 0 && file_size < size) {
+        error_setg(errp, "backing store %s size 0x%" PRIx64
+                   " does not match 'size' option 0x" RAM_ADDR_FMT,
+                   mem_path, file_size, size);
+        close(fd);
+        return NULL;
+    }
+
     new_block = g_malloc0(sizeof(*new_block));
     new_block->mr = mr;
     new_block->used_length = size;
     new_block->max_length = size;
     new_block->flags = share ? RAM_SHARED : 0;
-    new_block->host = file_ram_alloc(new_block, size,
-                                     mem_path, errp);
+    new_block->host = file_ram_alloc(new_block, size, fd, !file_size, errp);
     if (!new_block->host) {
+        if (created) {
+            unlink(mem_path);
+        }
+        close(fd);
         g_free(new_block);
         return NULL;
     }
--
1.8.3.1



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PULL 03/41] exec: split qemu_ram_alloc_from_file()

Paolo Bonzini-5
In reply to this post by Paolo Bonzini-5
From: Marc-André Lureau <[hidden email]>

Add qemu_ram_alloc_from_fd(), which can be use to allocate ramblock from
fd only.

Signed-off-by: Marc-André Lureau <[hidden email]>
Message-Id: <[hidden email]>
Signed-off-by: Paolo Bonzini <[hidden email]>
---
 exec.c                  | 45 ++++++++++++++++++++++++++++++---------------
 include/exec/ram_addr.h |  3 +++
 2 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/exec.c b/exec.c
index 42a5a7c..42ad1ea 100644
--- a/exec.c
+++ b/exec.c
@@ -1914,14 +1914,12 @@ static void ram_block_add(RAMBlock *new_block, Error **errp)
 }
 
 #ifdef __linux__
-RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
-                                   bool share, const char *mem_path,
-                                   Error **errp)
+RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr,
+                                 bool share, int fd,
+                                 Error **errp)
 {
     RAMBlock *new_block;
     Error *local_err = NULL;
-    int fd;
-    bool created;
     int64_t file_size;
 
     if (xen_enabled()) {
@@ -1946,18 +1944,12 @@ RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
         return NULL;
     }
 
-    fd = file_ram_open(mem_path, memory_region_name(mr), &created, errp);
-    if (fd < 0) {
-        return NULL;
-    }
-
     size = HOST_PAGE_ALIGN(size);
     file_size = get_file_size(fd);
     if (file_size > 0 && file_size < size) {
         error_setg(errp, "backing store %s size 0x%" PRIx64
                    " does not match 'size' option 0x" RAM_ADDR_FMT,
                    mem_path, file_size, size);
-        close(fd);
         return NULL;
     }
 
@@ -1968,10 +1960,6 @@ RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
     new_block->flags = share ? RAM_SHARED : 0;
     new_block->host = file_ram_alloc(new_block, size, fd, !file_size, errp);
     if (!new_block->host) {
-        if (created) {
-            unlink(mem_path);
-        }
-        close(fd);
         g_free(new_block);
         return NULL;
     }
@@ -1983,6 +1971,33 @@ RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
         return NULL;
     }
     return new_block;
+
+}
+
+
+RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
+                                   bool share, const char *mem_path,
+                                   Error **errp)
+{
+    int fd;
+    bool created;
+    RAMBlock *block;
+
+    fd = file_ram_open(mem_path, memory_region_name(mr), &created, errp);
+    if (fd < 0) {
+        return NULL;
+    }
+
+    block = qemu_ram_alloc_from_fd(size, mr, share, fd, errp);
+    if (!block) {
+        if (created) {
+            unlink(mem_path);
+        }
+        close(fd);
+        return NULL;
+    }
+
+    return block;
 }
 #endif
 
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 140efa8..73d1bea 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -65,6 +65,9 @@ unsigned long last_ram_page(void);
 RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
                                    bool share, const char *mem_path,
                                    Error **errp);
+RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr,
+                                 bool share, int fd,
+                                 Error **errp);
 RAMBlock *qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
                                   MemoryRegion *mr, Error **errp);
 RAMBlock *qemu_ram_alloc(ram_addr_t size, MemoryRegion *mr, Error **errp);
--
1.8.3.1



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PULL 04/41] Add memory_region_init_ram_from_fd()

Paolo Bonzini-5
In reply to this post by Paolo Bonzini-5
From: Marc-André Lureau <[hidden email]>

Add a new function to initialize a RAM memory region with a file
descriptor to be mmap-ed.

Signed-off-by: Marc-André Lureau <[hidden email]>
Message-Id: <[hidden email]>
Signed-off-by: Paolo Bonzini <[hidden email]>
---
 include/exec/memory.h | 20 ++++++++++++++++++++
 memory.c              | 16 ++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 80e605a..51a54c9 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -456,6 +456,26 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
                                       bool share,
                                       const char *path,
                                       Error **errp);
+
+/**
+ * memory_region_init_ram_from_fd:  Initialize RAM memory region with a
+ *                                  mmap-ed backend.
+ *
+ * @mr: the #MemoryRegion to be initialized.
+ * @owner: the object that tracks the region's reference count
+ * @name: the name of the region.
+ * @size: size of the region.
+ * @share: %true if memory must be mmaped with the MAP_SHARED flag
+ * @fd: the fd to mmap.
+ * @errp: pointer to Error*, to store an error if it happens.
+ */
+void memory_region_init_ram_from_fd(MemoryRegion *mr,
+                                    struct Object *owner,
+                                    const char *name,
+                                    uint64_t size,
+                                    bool share,
+                                    int fd,
+                                    Error **errp);
 #endif
 
 /**
diff --git a/memory.c b/memory.c
index 0ddc4cc..b2ace20 100644
--- a/memory.c
+++ b/memory.c
@@ -1397,6 +1397,22 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
     mr->ram_block = qemu_ram_alloc_from_file(size, mr, share, path, errp);
     mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
 }
+
+void memory_region_init_ram_from_fd(MemoryRegion *mr,
+                                    struct Object *owner,
+                                    const char *name,
+                                    uint64_t size,
+                                    bool share,
+                                    int fd,
+                                    Error **errp)
+{
+    memory_region_init(mr, owner, name, size);
+    mr->ram = true;
+    mr->terminates = true;
+    mr->destructor = memory_region_destructor_ram;
+    mr->ram_block = qemu_ram_alloc_from_fd(size, mr, share, fd, errp);
+    mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
+}
 #endif
 
 void memory_region_init_ram_ptr(MemoryRegion *mr,
--
1.8.3.1



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PULL 05/41] ivshmem: use ram_from_fd()

Paolo Bonzini-5
In reply to this post by Paolo Bonzini-5
From: Marc-André Lureau <[hidden email]>

Instead of having its own mmap handling code, reuse the code from
exec.c.

Note: memory_region_init_ram_from_fd() adds some restrictions
(check for xen, kvm sync-mmu, etc) and changes (such as size
alignment). This may actually be more correct.

Signed-off-by: Marc-André Lureau <[hidden email]>
Message-Id: <[hidden email]>
Signed-off-by: Paolo Bonzini <[hidden email]>
---
 hw/misc/ivshmem.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index 6367d04..2f0819d 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -491,9 +491,9 @@ static void setup_interrupt(IVShmemState *s, int vector, Error **errp)
 
 static void process_msg_shmem(IVShmemState *s, int fd, Error **errp)
 {
+    Error *local_err = NULL;
     struct stat buf;
     size_t size;
-    void *ptr;
 
     if (s->ivshmem_bar2) {
         error_setg(errp, "server sent unexpected shared memory message");
@@ -522,15 +522,13 @@ static void process_msg_shmem(IVShmemState *s, int fd, Error **errp)
     }
 
     /* mmap the region and map into the BAR2 */
-    ptr = mmap(0, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
-    if (ptr == MAP_FAILED) {
-        error_setg_errno(errp, errno, "Failed to mmap shared memory");
-        close(fd);
+    memory_region_init_ram_from_fd(&s->server_bar2, OBJECT(s),
+                                   "ivshmem.bar2", size, true, fd, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
         return;
     }
-    memory_region_init_ram_ptr(&s->server_bar2, OBJECT(s),
-                               "ivshmem.bar2", size, ptr);
-    memory_region_set_fd(&s->server_bar2, fd);
+
     s->ivshmem_bar2 = &s->server_bar2;
 }
 
--
1.8.3.1



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PULL 06/41] memory: remove memory_region_set_fd

Paolo Bonzini-5
In reply to this post by Paolo Bonzini-5
From: Marc-André Lureau <[hidden email]>

Now unnecessary since ivshmem uses memory_region_init_ram_from_fd.

Signed-off-by: Marc-André Lureau <[hidden email]>
Message-Id: <[hidden email]>
Signed-off-by: Paolo Bonzini <[hidden email]>
---
 include/exec/memory.h | 11 -----------
 memory.c              | 10 ----------
 2 files changed, 21 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 51a54c9..37f8e78 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -825,17 +825,6 @@ static inline bool memory_region_is_rom(MemoryRegion *mr)
 int memory_region_get_fd(MemoryRegion *mr);
 
 /**
- * memory_region_set_fd: Mark a RAM memory region as backed by a
- * file descriptor.
- *
- * This function is typically used after memory_region_init_ram_ptr().
- *
- * @mr: the memory region being queried.
- * @fd: the file descriptor that backs @mr.
- */
-void memory_region_set_fd(MemoryRegion *mr, int fd);
-
-/**
  * memory_region_from_host: Convert a pointer into a RAM memory region
  * and an offset within it.
  *
diff --git a/memory.c b/memory.c
index b2ace20..e08fa0a 100644
--- a/memory.c
+++ b/memory.c
@@ -1851,16 +1851,6 @@ int memory_region_get_fd(MemoryRegion *mr)
     return fd;
 }
 
-void memory_region_set_fd(MemoryRegion *mr, int fd)
-{
-    rcu_read_lock();
-    while (mr->alias) {
-        mr = mr->alias;
-    }
-    mr->ram_block->fd = fd;
-    rcu_read_unlock();
-}
-
 void *memory_region_get_ram_ptr(MemoryRegion *mr)
 {
     void *ptr;
--
1.8.3.1



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PULL 07/41] megasas: add qtest

Paolo Bonzini-5
In reply to this post by Paolo Bonzini-5
Signed-off-by: Paolo Bonzini <[hidden email]>
---
 tests/Makefile.include |  3 +++
 tests/megasas-test.c   | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+)
 create mode 100644 tests/megasas-test.c

diff --git a/tests/Makefile.include b/tests/Makefile.include
index f42f3df..77da9b7 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -205,6 +205,8 @@ check-qtest-pci-y += tests/intel-hda-test$(EXESUF)
 gcov-files-pci-y += hw/audio/intel-hda.c hw/audio/hda-codec.c
 check-qtest-pci-$(CONFIG_EVENTFD) += tests/ivshmem-test$(EXESUF)
 gcov-files-pci-y += hw/misc/ivshmem.c
+check-qtest-pci-y += tests/megasas-test$(EXESUF)
+gcov-files-pci-y += hw/scsi/megasas.c
 
 check-qtest-i386-y = tests/endianness-test$(EXESUF)
 check-qtest-i386-y += tests/fdc-test$(EXESUF)
@@ -755,6 +757,7 @@ tests/test-filter-mirror$(EXESUF): tests/test-filter-mirror.o $(qtest-obj-y)
 tests/test-filter-redirector$(EXESUF): tests/test-filter-redirector.o $(qtest-obj-y)
 tests/test-x86-cpuid-compat$(EXESUF): tests/test-x86-cpuid-compat.o $(qtest-obj-y)
 tests/ivshmem-test$(EXESUF): tests/ivshmem-test.o contrib/ivshmem-server/ivshmem-server.o $(libqos-pc-obj-y) $(libqos-spapr-obj-y)
+tests/megasas-test$(EXESUF): tests/megasas-test.o $(libqos-spapr-obj-y) $(libqos-pc-obj-y)
 tests/vhost-user-bridge$(EXESUF): tests/vhost-user-bridge.o contrib/libvhost-user/libvhost-user.o $(test-util-obj-y)
 tests/test-uuid$(EXESUF): tests/test-uuid.o $(test-util-obj-y)
 tests/test-arm-mptimer$(EXESUF): tests/test-arm-mptimer.o
diff --git a/tests/megasas-test.c b/tests/megasas-test.c
new file mode 100644
index 0000000..a9e56a2
--- /dev/null
+++ b/tests/megasas-test.c
@@ -0,0 +1,51 @@
+/*
+ * QTest testcase for LSI MegaRAID
+ *
+ * Copyright (c) 2017 Red Hat Inc.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+#include "qemu/bswap.h"
+#include "libqos/libqos-pc.h"
+#include "libqos/libqos-spapr.h"
+
+static QOSState *qmegasas_start(const char *extra_opts)
+{
+    const char *arch = qtest_get_arch();
+    const char *cmd = "-drive id=hd0,if=none,file=null-co://,format=raw "
+                      "-device megasas,id=scsi0,addr=04.0 "
+                      "-device scsi-hd,bus=scsi0.0,drive=hd0 %s";
+
+    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+        return qtest_pc_boot(cmd, extra_opts ? : "");
+    }
+
+    g_printerr("virtio-scsi tests are only available on x86 or ppc64\n");
+    exit(EXIT_FAILURE);
+}
+
+static void qmegasas_stop(QOSState *qs)
+{
+    qtest_shutdown(qs);
+}
+
+/* Tests only initialization so far. TODO: Replace with functional tests */
+static void pci_nop(void)
+{
+    QOSState *qs;
+
+    qs = qmegasas_start(NULL);
+    qmegasas_stop(qs);
+}
+
+int main(int argc, char **argv)
+{
+    g_test_init(&argc, &argv, NULL);
+    qtest_add_func("/megasas/pci/nop", pci_nop);
+
+    return g_test_run();
+}
--
1.8.3.1



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PULL 08/41] megasas: do not read sense length more than once from frame

Paolo Bonzini-5
In reply to this post by Paolo Bonzini-5
Avoid TOC-TOU bugs depending on how the compiler behaves.

Signed-off-by: Paolo Bonzini <[hidden email]>
---
 hw/scsi/megasas.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index 804122a..1888118 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -309,9 +309,11 @@ static int megasas_build_sense(MegasasCmd *cmd, uint8_t *sense_ptr,
     PCIDevice *pcid = PCI_DEVICE(cmd->state);
     uint32_t pa_hi = 0, pa_lo;
     hwaddr pa;
+    int frame_sense_len;
 
-    if (sense_len > cmd->frame->header.sense_len) {
-        sense_len = cmd->frame->header.sense_len;
+    frame_sense_len = cmd->frame->header.sense_len;
+    if (sense_len > frame_sense_len) {
+        sense_len = frame_sense_len;
     }
     if (sense_len) {
         pa_lo = le32_to_cpu(cmd->frame->pass.sense_addr_lo);
--
1.8.3.1



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PULL 09/41] megasas: do not read iovec count more than once from frame

Paolo Bonzini-5
In reply to this post by Paolo Bonzini-5
Avoid TOC-TOU bugs depending on how the compiler behaves.

Signed-off-by: Paolo Bonzini <[hidden email]>
---
 hw/scsi/megasas.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index 1888118..c353118 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -675,15 +675,16 @@ out:
 static int megasas_map_dcmd(MegasasState *s, MegasasCmd *cmd)
 {
     dma_addr_t iov_pa, iov_size;
+    int iov_count;
 
     cmd->flags = le16_to_cpu(cmd->frame->header.flags);
-    if (!cmd->frame->header.sge_count) {
+    iov_count = cmd->frame->header.sge_count;
+    if (!iov_count) {
         trace_megasas_dcmd_zero_sge(cmd->index);
         cmd->iov_size = 0;
         return 0;
-    } else if (cmd->frame->header.sge_count > 1) {
-        trace_megasas_dcmd_invalid_sge(cmd->index,
-                                       cmd->frame->header.sge_count);
+    } else if (iov_count > 1) {
+        trace_megasas_dcmd_invalid_sge(cmd->index, iov_count);
         cmd->iov_size = 0;
         return -EINVAL;
     }
--
1.8.3.1



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PULL 10/41] megasas: do not read DCMD opcode more than once from frame

Paolo Bonzini-5
In reply to this post by Paolo Bonzini-5
Avoid TOC-TOU bugs by storing the DCMD opcode in the MegasasCmd

Signed-off-by: Paolo Bonzini <[hidden email]>
---
 hw/scsi/megasas.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index c353118..a3f75c1 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -63,6 +63,7 @@ typedef struct MegasasCmd {
 
     hwaddr pa;
     hwaddr pa_size;
+    uint32_t dcmd_opcode;
     union mfi_frame *frame;
     SCSIRequest *req;
     QEMUSGList qsg;
@@ -513,6 +514,7 @@ static MegasasCmd *megasas_enqueue_frame(MegasasState *s,
         cmd->context &= (uint64_t)0xFFFFFFFF;
     }
     cmd->count = count;
+    cmd->dcmd_opcode = -1;
     s->busy++;
 
     if (s->consumer_pa) {
@@ -1562,22 +1564,21 @@ static const struct dcmd_cmd_tbl_t {
 
 static int megasas_handle_dcmd(MegasasState *s, MegasasCmd *cmd)
 {
-    int opcode;
     int retval = 0;
     size_t len;
     const struct dcmd_cmd_tbl_t *cmdptr = dcmd_cmd_tbl;
 
-    opcode = le32_to_cpu(cmd->frame->dcmd.opcode);
-    trace_megasas_handle_dcmd(cmd->index, opcode);
+    cmd->dcmd_opcode = le32_to_cpu(cmd->frame->dcmd.opcode);
+    trace_megasas_handle_dcmd(cmd->index, cmd->dcmd_opcode);
     if (megasas_map_dcmd(s, cmd) < 0) {
         return MFI_STAT_MEMORY_NOT_AVAILABLE;
     }
-    while (cmdptr->opcode != -1 && cmdptr->opcode != opcode) {
+    while (cmdptr->opcode != -1 && cmdptr->opcode != cmd->dcmd_opcode) {
         cmdptr++;
     }
     len = cmd->iov_size;
     if (cmdptr->opcode == -1) {
-        trace_megasas_dcmd_unhandled(cmd->index, opcode, len);
+        trace_megasas_dcmd_unhandled(cmd->index, cmd->dcmd_opcode, len);
         retval = megasas_dcmd_dummy(s, cmd);
     } else {
         trace_megasas_dcmd_enter(cmd->index, cmdptr->desc, len);
@@ -1592,13 +1593,11 @@ static int megasas_handle_dcmd(MegasasState *s, MegasasCmd *cmd)
 static int megasas_finish_internal_dcmd(MegasasCmd *cmd,
                                         SCSIRequest *req)
 {
-    int opcode;
     int retval = MFI_STAT_OK;
     int lun = req->lun;
 
-    opcode = le32_to_cpu(cmd->frame->dcmd.opcode);
-    trace_megasas_dcmd_internal_finish(cmd->index, opcode, lun);
-    switch (opcode) {
+    trace_megasas_dcmd_internal_finish(cmd->index, cmd->dcmd_opcode, lun);
+    switch (cmd->dcmd_opcode) {
     case MFI_DCMD_PD_GET_INFO:
         retval = megasas_pd_get_info_submit(req->dev, lun, cmd);
         break;
@@ -1606,7 +1605,7 @@ static int megasas_finish_internal_dcmd(MegasasCmd *cmd,
         retval = megasas_ld_get_info_submit(req->dev, lun, cmd);
         break;
     default:
-        trace_megasas_dcmd_internal_invalid(cmd->index, opcode);
+        trace_megasas_dcmd_internal_invalid(cmd->index, cmd->dcmd_opcode);
         retval = MFI_STAT_INVALID_DCMD;
         break;
     }
@@ -1827,7 +1826,6 @@ static void megasas_xfer_complete(SCSIRequest *req, uint32_t len)
 {
     MegasasCmd *cmd = req->hba_private;
     uint8_t *buf;
-    uint32_t opcode;
 
     trace_megasas_io_complete(cmd->index, len);
 
@@ -1837,8 +1835,7 @@ static void megasas_xfer_complete(SCSIRequest *req, uint32_t len)
     }
 
     buf = scsi_req_get_buf(req);
-    opcode = le32_to_cpu(cmd->frame->dcmd.opcode);
-    if (opcode == MFI_DCMD_PD_GET_INFO && cmd->iov_buf) {
+    if (cmd->dcmd_opcode == MFI_DCMD_PD_GET_INFO && cmd->iov_buf) {
         struct mfi_pd_info *info = cmd->iov_buf;
 
         if (info->inquiry_data[0] == 0x7f) {
@@ -1849,7 +1846,7 @@ static void megasas_xfer_complete(SCSIRequest *req, uint32_t len)
             memcpy(info->vpd_page83, buf, len);
         }
         scsi_req_continue(req);
-    } else if (opcode == MFI_DCMD_LD_GET_INFO) {
+    } else if (cmd->dcmd_opcode == MFI_DCMD_LD_GET_INFO) {
         struct mfi_ld_info *info = cmd->iov_buf;
 
         if (cmd->iov_buf) {
--
1.8.3.1



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PULL 11/41] megasas: do not read command more than once from frame

Paolo Bonzini-5
In reply to this post by Paolo Bonzini-5
Avoid TOC-TOU bugs by passing the frame_cmd down, and checking
cmd->dcmd_opcode instead of cmd->frame->header.frame_cmd.

Signed-off-by: Paolo Bonzini <[hidden email]>
---
 hw/scsi/megasas.c | 60 +++++++++++++++++++++++--------------------------------
 1 file changed, 25 insertions(+), 35 deletions(-)

diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index a3f75c1..38e0a2f 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -1591,12 +1591,13 @@ static int megasas_handle_dcmd(MegasasState *s, MegasasCmd *cmd)
 }
 
 static int megasas_finish_internal_dcmd(MegasasCmd *cmd,
-                                        SCSIRequest *req)
+                                        SCSIRequest *req, size_t resid)
 {
     int retval = MFI_STAT_OK;
     int lun = req->lun;
 
     trace_megasas_dcmd_internal_finish(cmd->index, cmd->dcmd_opcode, lun);
+    cmd->iov_size -= resid;
     switch (cmd->dcmd_opcode) {
     case MFI_DCMD_PD_GET_INFO:
         retval = megasas_pd_get_info_submit(req->dev, lun, cmd);
@@ -1649,11 +1650,12 @@ static int megasas_enqueue_req(MegasasCmd *cmd, bool is_write)
 }
 
 static int megasas_handle_scsi(MegasasState *s, MegasasCmd *cmd,
-                               bool is_logical)
+                               int frame_cmd)
 {
     uint8_t *cdb;
     bool is_write;
     struct SCSIDevice *sdev = NULL;
+    bool is_logical = (frame_cmd == MFI_CMD_LD_SCSI_IO);
 
     cdb = cmd->frame->pass.cdb;
 
@@ -1661,7 +1663,7 @@ static int megasas_handle_scsi(MegasasState *s, MegasasCmd *cmd,
         if (cmd->frame->header.target_id >= MFI_MAX_LD ||
             cmd->frame->header.lun_id != 0) {
             trace_megasas_scsi_target_not_present(
-                mfi_frame_desc[cmd->frame->header.frame_cmd], is_logical,
+                mfi_frame_desc[frame_cmd], is_logical,
                 cmd->frame->header.target_id, cmd->frame->header.lun_id);
             return MFI_STAT_DEVICE_NOT_FOUND;
         }
@@ -1671,19 +1673,20 @@ static int megasas_handle_scsi(MegasasState *s, MegasasCmd *cmd,
 
     cmd->iov_size = le32_to_cpu(cmd->frame->header.data_len);
     trace_megasas_handle_scsi(mfi_frame_desc[cmd->frame->header.frame_cmd],
-                              is_logical, cmd->frame->header.target_id,
+    trace_megasas_handle_scsi(mfi_frame_desc[frame_cmd], is_logical,
+                              cmd->frame->header.target_id,
                               cmd->frame->header.lun_id, sdev, cmd->iov_size);
 
     if (!sdev || (megasas_is_jbod(s) && is_logical)) {
         trace_megasas_scsi_target_not_present(
-            mfi_frame_desc[cmd->frame->header.frame_cmd], is_logical,
+            mfi_frame_desc[frame_cmd], is_logical,
             cmd->frame->header.target_id, cmd->frame->header.lun_id);
         return MFI_STAT_DEVICE_NOT_FOUND;
     }
 
     if (cmd->frame->header.cdb_len > 16) {
         trace_megasas_scsi_invalid_cdb_len(
-                mfi_frame_desc[cmd->frame->header.frame_cmd], is_logical,
+                mfi_frame_desc[frame_cmd], is_logical,
                 cmd->frame->header.target_id, cmd->frame->header.lun_id,
                 cmd->frame->header.cdb_len);
         megasas_write_sense(cmd, SENSE_CODE(INVALID_OPCODE));
@@ -1703,7 +1706,7 @@ static int megasas_handle_scsi(MegasasState *s, MegasasCmd *cmd,
                             cmd->frame->header.lun_id, cdb, cmd);
     if (!cmd->req) {
         trace_megasas_scsi_req_alloc_failed(
-                mfi_frame_desc[cmd->frame->header.frame_cmd],
+                mfi_frame_desc[frame_cmd],
                 cmd->frame->header.target_id, cmd->frame->header.lun_id);
         megasas_write_sense(cmd, SENSE_CODE(NO_SENSE));
         cmd->frame->header.scsi_status = BUSY;
@@ -1725,11 +1728,11 @@ static int megasas_handle_scsi(MegasasState *s, MegasasCmd *cmd,
     return MFI_STAT_INVALID_STATUS;
 }
 
-static int megasas_handle_io(MegasasState *s, MegasasCmd *cmd)
+static int megasas_handle_io(MegasasState *s, MegasasCmd *cmd, int frame_cmd)
 {
     uint32_t lba_count, lba_start_hi, lba_start_lo;
     uint64_t lba_start;
-    bool is_write = (cmd->frame->header.frame_cmd == MFI_CMD_LD_WRITE);
+    bool is_write = (frame_cmd == MFI_CMD_LD_WRITE);
     uint8_t cdb[16];
     int len;
     struct SCSIDevice *sdev = NULL;
@@ -1746,20 +1749,20 @@ static int megasas_handle_io(MegasasState *s, MegasasCmd *cmd)
     }
 
     trace_megasas_handle_io(cmd->index,
-                            mfi_frame_desc[cmd->frame->header.frame_cmd],
+                            mfi_frame_desc[frame_cmd],
                             cmd->frame->header.target_id,
                             cmd->frame->header.lun_id,
                             (unsigned long)lba_start, (unsigned long)lba_count);
     if (!sdev) {
         trace_megasas_io_target_not_present(cmd->index,
-            mfi_frame_desc[cmd->frame->header.frame_cmd],
+            mfi_frame_desc[frame_cmd],
             cmd->frame->header.target_id, cmd->frame->header.lun_id);
         return MFI_STAT_DEVICE_NOT_FOUND;
     }
 
     if (cmd->frame->header.cdb_len > 16) {
         trace_megasas_scsi_invalid_cdb_len(
-            mfi_frame_desc[cmd->frame->header.frame_cmd], 1,
+            mfi_frame_desc[frame_cmd], 1,
             cmd->frame->header.target_id, cmd->frame->header.lun_id,
             cmd->frame->header.cdb_len);
         megasas_write_sense(cmd, SENSE_CODE(INVALID_OPCODE));
@@ -1781,7 +1784,7 @@ static int megasas_handle_io(MegasasState *s, MegasasCmd *cmd)
                             cmd->frame->header.lun_id, cdb, cmd);
     if (!cmd->req) {
         trace_megasas_scsi_req_alloc_failed(
-            mfi_frame_desc[cmd->frame->header.frame_cmd],
+            mfi_frame_desc[frame_cmd],
             cmd->frame->header.target_id, cmd->frame->header.lun_id);
         megasas_write_sense(cmd, SENSE_CODE(NO_SENSE));
         cmd->frame->header.scsi_status = BUSY;
@@ -1799,23 +1802,11 @@ static int megasas_handle_io(MegasasState *s, MegasasCmd *cmd)
     return MFI_STAT_INVALID_STATUS;
 }
 
-static int megasas_finish_internal_command(MegasasCmd *cmd,
-                                           SCSIRequest *req, size_t resid)
-{
-    int retval = MFI_STAT_INVALID_CMD;
-
-    if (cmd->frame->header.frame_cmd == MFI_CMD_DCMD) {
-        cmd->iov_size -= resid;
-        retval = megasas_finish_internal_dcmd(cmd, req);
-    }
-    return retval;
-}
-
 static QEMUSGList *megasas_get_sg_list(SCSIRequest *req)
 {
     MegasasCmd *cmd = req->hba_private;
 
-    if (cmd->frame->header.frame_cmd == MFI_CMD_DCMD) {
+    if (cmd->dcmd_opcode != -1) {
         return NULL;
     } else {
         return &cmd->qsg;
@@ -1829,7 +1820,7 @@ static void megasas_xfer_complete(SCSIRequest *req, uint32_t len)
 
     trace_megasas_io_complete(cmd->index, len);
 
-    if (cmd->frame->header.frame_cmd != MFI_CMD_DCMD) {
+    if (cmd->dcmd_opcode != -1) {
         scsi_req_continue(req);
         return;
     }
@@ -1872,7 +1863,7 @@ static void megasas_command_complete(SCSIRequest *req, uint32_t status,
         /*
          * Internal command complete
          */
-        cmd_status = megasas_finish_internal_command(cmd, req, resid);
+        cmd_status = megasas_finish_internal_dcmd(cmd, req, resid);
         if (cmd_status == MFI_STAT_INVALID_STATUS) {
             return;
         }
@@ -1943,6 +1934,7 @@ static void megasas_handle_frame(MegasasState *s, uint64_t frame_addr,
 {
     uint8_t frame_status = MFI_STAT_INVALID_CMD;
     uint64_t frame_context;
+    int frame_cmd;
     MegasasCmd *cmd;
 
     /*
@@ -1961,7 +1953,8 @@ static void megasas_handle_frame(MegasasState *s, uint64_t frame_addr,
         s->event_count++;
         return;
     }
-    switch (cmd->frame->header.frame_cmd) {
+    frame_cmd = cmd->frame->header.frame_cmd;
+    switch (frame_cmd) {
     case MFI_CMD_INIT:
         frame_status = megasas_init_firmware(s, cmd);
         break;
@@ -1972,18 +1965,15 @@ static void megasas_handle_frame(MegasasState *s, uint64_t frame_addr,
         frame_status = megasas_handle_abort(s, cmd);
         break;
     case MFI_CMD_PD_SCSI_IO:
-        frame_status = megasas_handle_scsi(s, cmd, 0);
-        break;
     case MFI_CMD_LD_SCSI_IO:
-        frame_status = megasas_handle_scsi(s, cmd, 1);
+        frame_status = megasas_handle_scsi(s, cmd, frame_cmd);
         break;
     case MFI_CMD_LD_READ:
     case MFI_CMD_LD_WRITE:
-        frame_status = megasas_handle_io(s, cmd);
+        frame_status = megasas_handle_io(s, cmd, frame_cmd);
         break;
     default:
-        trace_megasas_unhandled_frame_cmd(cmd->index,
-                                          cmd->frame->header.frame_cmd);
+        trace_megasas_unhandled_frame_cmd(cmd->index, frame_cmd);
         s->event_count++;
         break;
     }
--
1.8.3.1



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PULL 12/41] megasas: do not read SCSI req parameters more than once from frame

Paolo Bonzini-5
In reply to this post by Paolo Bonzini-5
Signed-off-by: Paolo Bonzini <[hidden email]>
---
 hw/scsi/megasas.c | 60 ++++++++++++++++++++++++-------------------------------
 1 file changed, 26 insertions(+), 34 deletions(-)

diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index 38e0a2f..135662d 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -1653,42 +1653,39 @@ static int megasas_handle_scsi(MegasasState *s, MegasasCmd *cmd,
                                int frame_cmd)
 {
     uint8_t *cdb;
+    int target_id, lun_id, cdb_len;
     bool is_write;
     struct SCSIDevice *sdev = NULL;
     bool is_logical = (frame_cmd == MFI_CMD_LD_SCSI_IO);
 
     cdb = cmd->frame->pass.cdb;
+    target_id = cmd->frame->header.target_id;
+    lun_id = cmd->frame->header.lun_id;
+    cdb_len = cmd->frame->header.cdb_len;
 
     if (is_logical) {
-        if (cmd->frame->header.target_id >= MFI_MAX_LD ||
-            cmd->frame->header.lun_id != 0) {
+        if (target_id >= MFI_MAX_LD || lun_id != 0) {
             trace_megasas_scsi_target_not_present(
-                mfi_frame_desc[frame_cmd], is_logical,
-                cmd->frame->header.target_id, cmd->frame->header.lun_id);
+                mfi_frame_desc[frame_cmd], is_logical, target_id, lun_id);
             return MFI_STAT_DEVICE_NOT_FOUND;
         }
     }
-    sdev = scsi_device_find(&s->bus, 0, cmd->frame->header.target_id,
-                            cmd->frame->header.lun_id);
+    sdev = scsi_device_find(&s->bus, 0, target_id, lun_id);
 
     cmd->iov_size = le32_to_cpu(cmd->frame->header.data_len);
-    trace_megasas_handle_scsi(mfi_frame_desc[cmd->frame->header.frame_cmd],
     trace_megasas_handle_scsi(mfi_frame_desc[frame_cmd], is_logical,
-                              cmd->frame->header.target_id,
-                              cmd->frame->header.lun_id, sdev, cmd->iov_size);
+                              target_id, lun_id, sdev, cmd->iov_size);
 
     if (!sdev || (megasas_is_jbod(s) && is_logical)) {
         trace_megasas_scsi_target_not_present(
-            mfi_frame_desc[frame_cmd], is_logical,
-            cmd->frame->header.target_id, cmd->frame->header.lun_id);
+            mfi_frame_desc[frame_cmd], is_logical, target_id, lun_id);
         return MFI_STAT_DEVICE_NOT_FOUND;
     }
 
-    if (cmd->frame->header.cdb_len > 16) {
+    if (cdb_len > 16) {
         trace_megasas_scsi_invalid_cdb_len(
                 mfi_frame_desc[frame_cmd], is_logical,
-                cmd->frame->header.target_id, cmd->frame->header.lun_id,
-                cmd->frame->header.cdb_len);
+                target_id, lun_id, cdb_len);
         megasas_write_sense(cmd, SENSE_CODE(INVALID_OPCODE));
         cmd->frame->header.scsi_status = CHECK_CONDITION;
         s->event_count++;
@@ -1702,12 +1699,10 @@ static int megasas_handle_scsi(MegasasState *s, MegasasCmd *cmd,
         return MFI_STAT_SCSI_DONE_WITH_ERROR;
     }
 
-    cmd->req = scsi_req_new(sdev, cmd->index,
-                            cmd->frame->header.lun_id, cdb, cmd);
+    cmd->req = scsi_req_new(sdev, cmd->index, lun_id, cdb, cmd);
     if (!cmd->req) {
         trace_megasas_scsi_req_alloc_failed(
-                mfi_frame_desc[frame_cmd],
-                cmd->frame->header.target_id, cmd->frame->header.lun_id);
+                mfi_frame_desc[frame_cmd], target_id, lun_id);
         megasas_write_sense(cmd, SENSE_CODE(NO_SENSE));
         cmd->frame->header.scsi_status = BUSY;
         s->event_count++;
@@ -1736,35 +1731,33 @@ static int megasas_handle_io(MegasasState *s, MegasasCmd *cmd, int frame_cmd)
     uint8_t cdb[16];
     int len;
     struct SCSIDevice *sdev = NULL;
+    int target_id, lun_id, cdb_len;
 
     lba_count = le32_to_cpu(cmd->frame->io.header.data_len);
     lba_start_lo = le32_to_cpu(cmd->frame->io.lba_lo);
     lba_start_hi = le32_to_cpu(cmd->frame->io.lba_hi);
     lba_start = ((uint64_t)lba_start_hi << 32) | lba_start_lo;
 
-    if (cmd->frame->header.target_id < MFI_MAX_LD &&
-        cmd->frame->header.lun_id == 0) {
-        sdev = scsi_device_find(&s->bus, 0, cmd->frame->header.target_id,
-                                cmd->frame->header.lun_id);
+    target_id = cmd->frame->header.target_id;
+    lun_id = cmd->frame->header.lun_id;
+    cdb_len = cmd->frame->header.cdb_len;
+
+    if (target_id < MFI_MAX_LD && lun_id == 0) {
+        sdev = scsi_device_find(&s->bus, 0, target_id, lun_id);
     }
 
     trace_megasas_handle_io(cmd->index,
-                            mfi_frame_desc[frame_cmd],
-                            cmd->frame->header.target_id,
-                            cmd->frame->header.lun_id,
+                            mfi_frame_desc[frame_cmd], target_id, lun_id,
                             (unsigned long)lba_start, (unsigned long)lba_count);
     if (!sdev) {
         trace_megasas_io_target_not_present(cmd->index,
-            mfi_frame_desc[frame_cmd],
-            cmd->frame->header.target_id, cmd->frame->header.lun_id);
+            mfi_frame_desc[frame_cmd], target_id, lun_id);
         return MFI_STAT_DEVICE_NOT_FOUND;
     }
 
-    if (cmd->frame->header.cdb_len > 16) {
+    if (cdb_len > 16) {
         trace_megasas_scsi_invalid_cdb_len(
-            mfi_frame_desc[frame_cmd], 1,
-            cmd->frame->header.target_id, cmd->frame->header.lun_id,
-            cmd->frame->header.cdb_len);
+            mfi_frame_desc[frame_cmd], 1, target_id, lun_id, cdb_len);
         megasas_write_sense(cmd, SENSE_CODE(INVALID_OPCODE));
         cmd->frame->header.scsi_status = CHECK_CONDITION;
         s->event_count++;
@@ -1781,11 +1774,10 @@ static int megasas_handle_io(MegasasState *s, MegasasCmd *cmd, int frame_cmd)
 
     megasas_encode_lba(cdb, lba_start, lba_count, is_write);
     cmd->req = scsi_req_new(sdev, cmd->index,
-                            cmd->frame->header.lun_id, cdb, cmd);
+                            lun_id, cdb, cmd);
     if (!cmd->req) {
         trace_megasas_scsi_req_alloc_failed(
-            mfi_frame_desc[frame_cmd],
-            cmd->frame->header.target_id, cmd->frame->header.lun_id);
+            mfi_frame_desc[frame_cmd], target_id, lun_id);
         megasas_write_sense(cmd, SENSE_CODE(NO_SENSE));
         cmd->frame->header.scsi_status = BUSY;
         s->event_count++;
--
1.8.3.1



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PULL 13/41] megasas: always store SCSIRequest* into MegasasCmd

Paolo Bonzini-5
In reply to this post by Paolo Bonzini-5
This ensures that the request is unref'ed properly, and avoids a
segmentation fault in the new qtest testcase that is added.
This is CVE-2017-9503.

Reported-by: Zhangyanyu <[hidden email]>
Signed-off-by: Paolo Bonzini <[hidden email]>
---
 hw/scsi/megasas.c    | 31 ++++++++++++++++---------------
 tests/megasas-test.c | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 15 deletions(-)

diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index 135662d..734fdae 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -609,6 +609,9 @@ static void megasas_reset_frames(MegasasState *s)
 static void megasas_abort_command(MegasasCmd *cmd)
 {
     /* Never abort internal commands.  */
+    if (cmd->dcmd_opcode != -1) {
+        return;
+    }
     if (cmd->req != NULL) {
         scsi_req_cancel(cmd->req);
     }
@@ -1017,7 +1020,6 @@ static int megasas_pd_get_info_submit(SCSIDevice *sdev, int lun,
     uint64_t pd_size;
     uint16_t pd_id = ((sdev->id & 0xFF) << 8) | (lun & 0xFF);
     uint8_t cmdbuf[6];
-    SCSIRequest *req;
     size_t len, resid;
 
     if (!cmd->iov_buf) {
@@ -1026,8 +1028,8 @@ static int megasas_pd_get_info_submit(SCSIDevice *sdev, int lun,
         info->inquiry_data[0] = 0x7f; /* Force PQual 0x3, PType 0x1f */
         info->vpd_page83[0] = 0x7f;
         megasas_setup_inquiry(cmdbuf, 0, sizeof(info->inquiry_data));
-        req = scsi_req_new(sdev, cmd->index, lun, cmdbuf, cmd);
-        if (!req) {
+        cmd->req = scsi_req_new(sdev, cmd->index, lun, cmdbuf, cmd);
+        if (!cmd->req) {
             trace_megasas_dcmd_req_alloc_failed(cmd->index,
                                                 "PD get info std inquiry");
             g_free(cmd->iov_buf);
@@ -1036,26 +1038,26 @@ static int megasas_pd_get_info_submit(SCSIDevice *sdev, int lun,
         }
         trace_megasas_dcmd_internal_submit(cmd->index,
                                            "PD get info std inquiry", lun);
-        len = scsi_req_enqueue(req);
+        len = scsi_req_enqueue(cmd->req);
         if (len > 0) {
             cmd->iov_size = len;
-            scsi_req_continue(req);
+            scsi_req_continue(cmd->req);
         }
         return MFI_STAT_INVALID_STATUS;
     } else if (info->inquiry_data[0] != 0x7f && info->vpd_page83[0] == 0x7f) {
         megasas_setup_inquiry(cmdbuf, 0x83, sizeof(info->vpd_page83));
-        req = scsi_req_new(sdev, cmd->index, lun, cmdbuf, cmd);
-        if (!req) {
+        cmd->req = scsi_req_new(sdev, cmd->index, lun, cmdbuf, cmd);
+        if (!cmd->req) {
             trace_megasas_dcmd_req_alloc_failed(cmd->index,
                                                 "PD get info vpd inquiry");
             return MFI_STAT_FLASH_ALLOC_FAIL;
         }
         trace_megasas_dcmd_internal_submit(cmd->index,
                                            "PD get info vpd inquiry", lun);
-        len = scsi_req_enqueue(req);
+        len = scsi_req_enqueue(cmd->req);
         if (len > 0) {
             cmd->iov_size = len;
-            scsi_req_continue(req);
+            scsi_req_continue(cmd->req);
         }
         return MFI_STAT_INVALID_STATUS;
     }
@@ -1217,7 +1219,6 @@ static int megasas_ld_get_info_submit(SCSIDevice *sdev, int lun,
     struct mfi_ld_info *info = cmd->iov_buf;
     size_t dcmd_size = sizeof(struct mfi_ld_info);
     uint8_t cdb[6];
-    SCSIRequest *req;
     ssize_t len, resid;
     uint16_t sdev_id = ((sdev->id & 0xFF) << 8) | (lun & 0xFF);
     uint64_t ld_size;
@@ -1226,8 +1227,8 @@ static int megasas_ld_get_info_submit(SCSIDevice *sdev, int lun,
         cmd->iov_buf = g_malloc0(dcmd_size);
         info = cmd->iov_buf;
         megasas_setup_inquiry(cdb, 0x83, sizeof(info->vpd_page83));
-        req = scsi_req_new(sdev, cmd->index, lun, cdb, cmd);
-        if (!req) {
+        cmd->req = scsi_req_new(sdev, cmd->index, lun, cdb, cmd);
+        if (!cmd->req) {
             trace_megasas_dcmd_req_alloc_failed(cmd->index,
                                                 "LD get info vpd inquiry");
             g_free(cmd->iov_buf);
@@ -1236,10 +1237,10 @@ static int megasas_ld_get_info_submit(SCSIDevice *sdev, int lun,
         }
         trace_megasas_dcmd_internal_submit(cmd->index,
                                            "LD get info vpd inquiry", lun);
-        len = scsi_req_enqueue(req);
+        len = scsi_req_enqueue(cmd->req);
         if (len > 0) {
             cmd->iov_size = len;
-            scsi_req_continue(req);
+            scsi_req_continue(cmd->req);
         }
         return MFI_STAT_INVALID_STATUS;
     }
@@ -1851,7 +1852,7 @@ static void megasas_command_complete(SCSIRequest *req, uint32_t status,
         return;
     }
 
-    if (cmd->req == NULL) {
+    if (cmd->dcmd_opcode != -1) {
         /*
          * Internal command complete
          */
diff --git a/tests/megasas-test.c b/tests/megasas-test.c
index a9e56a2..ce960e7 100644
--- a/tests/megasas-test.c
+++ b/tests/megasas-test.c
@@ -42,10 +42,45 @@ static void pci_nop(void)
     qmegasas_stop(qs);
 }
 
+/* This used to cause a NULL pointer dereference.  */
+static void megasas_pd_get_info_fuzz(void)
+{
+    QPCIDevice *dev;
+    QOSState *qs;
+    QPCIBar bar;
+    uint32_t context[256];
+    uint64_t context_pa;
+    int i;
+
+    qs = qmegasas_start(NULL);
+    dev = qpci_device_find(qs->pcibus, QPCI_DEVFN(4,0));
+    g_assert(dev != NULL);
+
+    qpci_device_enable(dev);
+    bar = qpci_iomap(dev, 0, NULL);
+
+    memset(context, 0, sizeof(context));
+    context[0] = cpu_to_le32(0x05050505);
+    context[1] = cpu_to_le32(0x01010101);
+    for (i = 2; i < ARRAY_SIZE(context); i++) {
+        context[i] = cpu_to_le32(0x41414141);
+    }
+    context[6] = cpu_to_le32(0x02020000);
+    context[7] = cpu_to_le32(0);
+
+    context_pa = qmalloc(qs, sizeof(context));
+    memwrite(context_pa, context, sizeof(context));
+    qpci_io_writel(dev, bar, 0x40, context_pa);
+
+    g_free(dev);
+    qmegasas_stop(qs);
+}
+
 int main(int argc, char **argv)
 {
     g_test_init(&argc, &argv, NULL);
     qtest_add_func("/megasas/pci/nop", pci_nop);
+    qtest_add_func("/megasas/dcmd/pd-get-info/fuzz", megasas_pd_get_info_fuzz);
 
     return g_test_run();
 }
--
1.8.3.1



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PULL 14/41] Makefile: Do not generate files if "configure" has not been run yet

Paolo Bonzini-5
In reply to this post by Paolo Bonzini-5
From: Thomas Huth <[hidden email]>

When doing a "make -j10" in the vanilla QEMU source tree (without
running "configure" first), the Makefile currently generates two
files already, qemu-version.h and qemu-options.def. This should not
happen, so let's only build the generated files if config-host.mak
is available (i.e. "configure" has been run already).

Signed-off-by: Thomas Huth <[hidden email]>
Message-Id: <[hidden email]>
Signed-off-by: Paolo Bonzini <[hidden email]>
---
 Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Makefile b/Makefile
index c830d7a..32d4441 100644
--- a/Makefile
+++ b/Makefile
@@ -791,9 +791,11 @@ endif # CONFIG_WIN
 
 # Add a dependency on the generated files, so that they are always
 # rebuilt before other object files
+ifneq ($(wildcard config-host.mak),)
 ifneq ($(filter-out $(UNCHECKED_GOALS),$(MAKECMDGOALS)),$(if $(MAKECMDGOALS),,fail))
 Makefile: $(GENERATED_FILES)
 endif
+endif
 
 .SECONDARY: $(TRACE_HEADERS) $(TRACE_HEADERS:%=%-timestamp) \
  $(TRACE_SOURCES) $(TRACE_SOURCES:%=%-timestamp) \
--
1.8.3.1



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PULL 15/41] vl: Fix broken thread=xxx option of the --accel parameter

Paolo Bonzini-5
In reply to this post by Paolo Bonzini-5
From: Thomas Huth <[hidden email]>

Commit bde4d9205 ("Fix the -accel parameter and the documentation for
'hax'") introduced a regression by adding a new local accel_opts
variable which shadows the variable with the same name that is
declared at the beginning of the main() scope. This causes the
qemu_tcg_configure() call later to be always called with NULL, so
that the thread=xxx option gets ignored. Fix it by removing the
local accel_opts variable and use "opts" instead, which is meant
for storing temporary QemuOpts values.
And while we're at it, also change the exit(1) here to exit(0)
since asking for help is not an error.

Fixes: bde4d9205ee9def98852ff6054cdef4efd74e1f8
Reported-by: Markus Armbruster <[hidden email]>
Reported-by: Emilio G. Cota <[hidden email]>
Signed-off-by: Thomas Huth <[hidden email]>
Message-Id: <[hidden email]>
Signed-off-by: Paolo Bonzini <[hidden email]>
---
 vl.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/vl.c b/vl.c
index 32db19e..59fea15 100644
--- a/vl.c
+++ b/vl.c
@@ -3757,21 +3757,18 @@ int main(int argc, char **argv, char **envp)
                 qdev_prop_register_global(&kvm_pit_lost_tick_policy);
                 break;
             }
-            case QEMU_OPTION_accel: {
-                QemuOpts *accel_opts;
-
+            case QEMU_OPTION_accel:
                 accel_opts = qemu_opts_parse_noisily(qemu_find_opts("accel"),
                                                      optarg, true);
                 optarg = qemu_opt_get(accel_opts, "accel");
                 if (!optarg || is_help_option(optarg)) {
                     error_printf("Possible accelerators: kvm, xen, hax, tcg\n");
-                    exit(1);
+                    exit(0);
                 }
-                accel_opts = qemu_opts_create(qemu_find_opts("machine"), NULL,
-                                              false, &error_abort);
-                qemu_opt_set(accel_opts, "accel", optarg, &error_abort);
+                opts = qemu_opts_create(qemu_find_opts("machine"), NULL,
+                                        false, &error_abort);
+                qemu_opt_set(opts, "accel", optarg, &error_abort);
                 break;
-            }
             case QEMU_OPTION_usb:
                 olist = qemu_find_opts("machine");
                 qemu_opts_parse_noisily(olist, "usb=on", false);
--
1.8.3.1



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PULL 16/41] kvm-all: make async_safe_run_on_cpu safe on kvm too

Paolo Bonzini-5
In reply to this post by Paolo Bonzini-5
From: Roman Kagan <[hidden email]>

Wrap the bulk of kvm_cpu_exec with cpu_exec_start/end, so that kvm
version can also enjoy performing certain operations while all vCPUs are
quiescent.

Signed-off-by: Roman Kagan <[hidden email]>
Message-Id: <[hidden email]>
Signed-off-by: Paolo Bonzini <[hidden email]>
---
 kvm-all.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kvm-all.c b/kvm-all.c
index ab8262f..98ad151 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1977,6 +1977,7 @@ int kvm_cpu_exec(CPUState *cpu)
     }
 
     qemu_mutex_unlock_iothread();
+    cpu_exec_start(cpu);
 
     do {
         MemTxAttrs attrs;
@@ -2106,6 +2107,7 @@ int kvm_cpu_exec(CPUState *cpu)
         }
     } while (ret == 0);
 
+    cpu_exec_end(cpu);
     qemu_mutex_lock_iothread();
 
     if (ret < 0) {
--
1.8.3.1



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PULL 17/41] hax-all: make async_safe_run_on_cpu safe on HAX too

Paolo Bonzini-5
In reply to this post by Paolo Bonzini-5
While at it, drop the current_cpu assignment since this is a
per-thread variable on modern QEMU.

Cc: Vincent Palatin <[hidden email]>
Signed-off-by: Paolo Bonzini <[hidden email]>
---
 target/i386/hax-all.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/i386/hax-all.c b/target/i386/hax-all.c
index 097db5c..ba6117d 100644
--- a/target/i386/hax-all.c
+++ b/target/i386/hax-all.c
@@ -514,9 +514,10 @@ static int hax_vcpu_hax_exec(CPUArchState *env)
         hax_vcpu_interrupt(env);
 
         qemu_mutex_unlock_iothread();
+        cpu_exec_start(cpu);
         hax_ret = hax_vcpu_run(vcpu);
+        cpu_exec_end(cpu);
         qemu_mutex_lock_iothread();
-        current_cpu = cpu;
 
         /* Simply continue the vcpu_run if system call interrupted */
         if (hax_ret == -EINTR || hax_ret == -EAGAIN) {
--
1.8.3.1



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PULL 18/41] nbd: Fix regression on resiliency to port scan

Paolo Bonzini-5
In reply to this post by Paolo Bonzini-5
From: Eric Blake <[hidden email]>

Back in qemu 2.5, qemu-nbd was immune to port probes (a transient
server would not quit, regardless of how many probe connections
came and went, until a connection actually negotiated).  But we
broke that in commit ee7d7aa when removing the return value to
nbd_client_new(), although that patch also introduced a bug causing
an assertion failure on a client that fails negotiation.  We then
made it worse during refactoring in commit 1a6245a (a segfault
before we could even assert); the (masked) assertion was cleaned
up in d3780c2 (still in 2.6), and just recently we finally fixed
the segfault ("nbd: Fully intialize client in case of failed
negotiation").  But that still means that ever since we added
TLS support to qemu-nbd, we have been vulnerable to an ill-timed
port-scan being able to cause a denial of service by taking down
qemu-nbd before a real client has a chance to connect.

Since negotiation is now handled asynchronously via coroutines,
we no longer have a synchronous point of return by re-adding a
return value to nbd_client_new().  So this patch instead wires
things up to pass the negotiation status through the close_fn
callback function.

Simple test across two terminals:
$ qemu-nbd -f raw -p 30001 file
$ nmap 127.0.0.1 -p 30001 && \
  qemu-io -c 'r 0 512' -f raw nbd://localhost:30001

Note that this patch does not change what constitutes successful
negotiation (thus, a client must enter transmission phase before
that client can be considered as a reason to terminate the server
when the connection ends).  Perhaps we may want to tweak things
in a later patch to also treat a client that uses NBD_OPT_ABORT
as being a 'successful' negotiation (the client correctly talked
the NBD protocol, and informed us it was not going to use our
export after all), but that's a discussion for another day.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1451614

Signed-off-by: Eric Blake <[hidden email]>
Message-Id: <[hidden email]>
Signed-off-by: Paolo Bonzini <[hidden email]>
---
 blockdev-nbd.c      |  6 +++++-
 include/block/nbd.h |  2 +-
 nbd/server.c        | 24 +++++++++++++++---------
 qemu-nbd.c          |  4 ++--
 4 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index dd0860f..28f551a 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -27,6 +27,10 @@ typedef struct NBDServerData {
 
 static NBDServerData *nbd_server;
 
+static void nbd_blockdev_client_closed(NBDClient *client, bool ignored)
+{
+    nbd_client_put(client);
+}
 
 static gboolean nbd_accept(QIOChannel *ioc, GIOCondition condition,
                            gpointer opaque)
@@ -46,7 +50,7 @@ static gboolean nbd_accept(QIOChannel *ioc, GIOCondition condition,
     qio_channel_set_name(QIO_CHANNEL(cioc), "nbd-server");
     nbd_client_new(NULL, cioc,
                    nbd_server->tlscreds, NULL,
-                   nbd_client_put);
+                   nbd_blockdev_client_closed);
     object_unref(OBJECT(cioc));
     return TRUE;
 }
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 416257a..8fa5ce5 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -162,7 +162,7 @@ void nbd_client_new(NBDExport *exp,
                     QIOChannelSocket *sioc,
                     QCryptoTLSCreds *tlscreds,
                     const char *tlsaclname,
-                    void (*close)(NBDClient *));
+                    void (*close_fn)(NBDClient *, bool));
 void nbd_client_get(NBDClient *client);
 void nbd_client_put(NBDClient *client);
 
diff --git a/nbd/server.c b/nbd/server.c
index 49b55f6..f2b1aa4 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -81,7 +81,7 @@ static QTAILQ_HEAD(, NBDExport) exports = QTAILQ_HEAD_INITIALIZER(exports);
 
 struct NBDClient {
     int refcount;
-    void (*close)(NBDClient *client);
+    void (*close_fn)(NBDClient *client, bool negotiated);
 
     bool no_zeroes;
     NBDExport *exp;
@@ -778,7 +778,7 @@ void nbd_client_put(NBDClient *client)
     }
 }
 
-static void client_close(NBDClient *client)
+static void client_close(NBDClient *client, bool negotiated)
 {
     if (client->closing) {
         return;
@@ -793,8 +793,8 @@ static void client_close(NBDClient *client)
                          NULL);
 
     /* Also tell the client, so that they release their reference.  */
-    if (client->close) {
-        client->close(client);
+    if (client->close_fn) {
+        client->close_fn(client, negotiated);
     }
 }
 
@@ -975,7 +975,7 @@ void nbd_export_close(NBDExport *exp)
 
     nbd_export_get(exp);
     QTAILQ_FOREACH_SAFE(client, &exp->clients, next, next) {
-        client_close(client);
+        client_close(client, true);
     }
     nbd_export_set_name(exp, NULL);
     nbd_export_set_description(exp, NULL);
@@ -1337,7 +1337,7 @@ done:
 
 out:
     nbd_request_put(req);
-    client_close(client);
+    client_close(client, true);
     nbd_client_put(client);
 }
 
@@ -1363,7 +1363,7 @@ static coroutine_fn void nbd_co_client_start(void *opaque)
     qemu_co_mutex_init(&client->send_lock);
 
     if (nbd_negotiate(data)) {
-        client_close(client);
+        client_close(client, false);
         goto out;
     }
 
@@ -1373,11 +1373,17 @@ out:
     g_free(data);
 }
 
+/*
+ * Create a new client listener on the given export @exp, using the
+ * given channel @sioc.  Begin servicing it in a coroutine.  When the
+ * connection closes, call @close_fn with an indication of whether the
+ * client completed negotiation.
+ */
 void nbd_client_new(NBDExport *exp,
                     QIOChannelSocket *sioc,
                     QCryptoTLSCreds *tlscreds,
                     const char *tlsaclname,
-                    void (*close_fn)(NBDClient *))
+                    void (*close_fn)(NBDClient *, bool))
 {
     NBDClient *client;
     NBDClientNewData *data = g_new(NBDClientNewData, 1);
@@ -1394,7 +1400,7 @@ void nbd_client_new(NBDExport *exp,
     object_ref(OBJECT(client->sioc));
     client->ioc = QIO_CHANNEL(sioc);
     object_ref(OBJECT(client->ioc));
-    client->close = close_fn;
+    client->close_fn = close_fn;
 
     data->client = client;
     data->co = qemu_coroutine_create(nbd_co_client_start, data);
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 651f85e..9464a04 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -336,10 +336,10 @@ static void nbd_export_closed(NBDExport *exp)
 
 static void nbd_update_server_watch(void);
 
-static void nbd_client_closed(NBDClient *client)
+static void nbd_client_closed(NBDClient *client, bool negotiated)
 {
     nb_fds--;
-    if (nb_fds == 0 && !persistent && state == RUNNING) {
+    if (negotiated && nb_fds == 0 && !persistent && state == RUNNING) {
         state = TERMINATE;
     }
     nbd_update_server_watch();
--
1.8.3.1



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PULL 19/41] qemu-nbd: Ignore SIGPIPE

Paolo Bonzini-5
In reply to this post by Paolo Bonzini-5
From: Max Reitz <[hidden email]>

qemu proper has done so for 13 years
(8a7ddc38a60648257dc0645ab4a05b33d6040063), qemu-img and qemu-io have
done so for four years (526eda14a68d5b3596be715505289b541288ef2a).
Ignoring this signal is especially important in qemu-nbd because
otherwise a client can easily take down the qemu-nbd server by dropping
the connection when the server wants to send something, for example:

$ qemu-nbd -x foo -f raw -t null-co:// &
[1] 12726
$ qemu-io -c quit nbd://localhost/bar
can't open device nbd://localhost/bar: No export with name 'bar' available
[1]  + 12726 broken pipe  qemu-nbd -x foo -f raw -t null-co://

In this case, the client sends an NBD_OPT_ABORT and closes the
connection (because it is not required to wait for a reply), but the
server replies with an NBD_REP_ACK (because it is required to reply).

Signed-off-by: Max Reitz <[hidden email]>
Message-Id: <[hidden email]>
Signed-off-by: Paolo Bonzini <[hidden email]>
---
 qemu-nbd.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index 9464a04..4dd3fd4 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -581,6 +581,10 @@ int main(int argc, char **argv)
     sa_sigterm.sa_handler = termsig_handler;
     sigaction(SIGTERM, &sa_sigterm, NULL);
 
+#ifdef CONFIG_POSIX
+    signal(SIGPIPE, SIG_IGN);
+#endif
+
     module_call_init(MODULE_INIT_TRACE);
     qcrypto_init(&error_fatal);
 
--
1.8.3.1



123
Loading...