[Qemu-devel] [PULL 0/4] QEMU changes for 2.5.0-rc3

[Qemu-devel] [PULL 0/4] QEMU changes for 2.5.0-rc3

[Paolo Bonzini]

The following changes since commit 9d7b969ea6d9663a94760c6c131481b366f4d38a:

  Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20151201' into staging (2015-12-02 10:16:53 +0000)

are available in the git repository at:

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

for you to fetch changes up to 0c2d70c448b7853a91cfa63659aa3cc6630fb9be:

  translate-all: ensure host page mask is always extended with 1's (2015-12-02 13:12:30 +0100)

----------------------------------------------------------------
* exec.c use after free
* Xen 32-on-64 breakage
* missing EINTR
* naughty warning under qtest

----------------------------------------------------------------
Don Slutz (1):
      exec: Stop using memory after free

Michael S. Tsirkin (1):
      main-loop: suppress warnings under qtest

Paolo Bonzini (2):
      qemu-char: retry g_poll on EINTR
      translate-all: ensure host page mask is always extended with 1's

 bsd-user/elfload.c     | 3 +--
 exec.c                 | 4 +++-
 include/exec/cpu-all.h | 8 +++++---
 linux-user/elfload.c   | 3 +--
 linux-user/mmap.c      | 4 +---
 main-loop.c            | 2 +-
 qemu-char.c            | 7 ++++++-
 stubs/qtest.c          | 5 +++++
 translate-all.c        | 6 +++---
 translate-common.c     | 2 +-
 10 files changed, 27 insertions(+), 17 deletions(-)
-- 
2.5.0

[Qemu-devel] [PULL 1/4] exec: Stop using memory after free

[Paolo Bonzini]

From: Don Slutz <don.slutz@gmail.com>

memory_region_unref(mr) can free memory.

For example I got:

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7f43280d4700 (LWP 4462)]
0x00007f43323283c0 in phys_section_destroy (mr=0x7f43259468b0)
    at /home/don/xen/tools/qemu-xen-dir/exec.c:1023
1023        if (mr->subpage) {
(gdb) bt
    at /home/don/xen/tools/qemu-xen-dir/exec.c:1023
    at /home/don/xen/tools/qemu-xen-dir/exec.c:1034
    at /home/don/xen/tools/qemu-xen-dir/exec.c:2205
(gdb) p mr
$1 = (MemoryRegion *) 0x7f43259468b0

And this change prevents this.

Signed-off-by: Don Slutz <Don.Slutz@Gmail.com>
Message-Id: <1448921464-21845-1-git-send-email-Don.Slutz@Gmail.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 exec.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/exec.c b/exec.c
index de1cf19..0bf0a6e 100644
--- a/exec.c
+++ b/exec.c
@@ -1064,9 +1064,11 @@ static uint16_t phys_section_add(PhysPageMap *map,
 
 static void phys_section_destroy(MemoryRegion *mr)
 {
+    bool have_sub_page = mr->subpage;
+
     memory_region_unref(mr);
 
-    if (mr->subpage) {
+    if (have_sub_page) {
         subpage_t *subpage = container_of(mr, subpage_t, iomem);
         object_unref(OBJECT(&subpage->iomem));
         g_free(subpage);
-- 
2.5.0

[Qemu-devel] [PULL 2/4] qemu-char: retry g_poll on EINTR

[Paolo Bonzini]

This is a case where pty_chr_update_read_handler_locked's lack
of error checking can produce incorrect values.  We are not using
SIGUSR1 anymore, so this is quite theoretical, but easy to fix.

Reported-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-char.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/qemu-char.c b/qemu-char.c
index 5448b0f..2969c44 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1241,11 +1241,16 @@ static void pty_chr_update_read_handler_locked(CharDriverState *chr)
 {
     PtyCharDriver *s = chr->opaque;
     GPollFD pfd;
+    int rc;
 
     pfd.fd = g_io_channel_unix_get_fd(s->fd);
     pfd.events = G_IO_OUT;
     pfd.revents = 0;
-    g_poll(&pfd, 1, 0);
+    do {
+        rc = g_poll(&pfd, 1, 0);
+    } while (rc == -1 && errno == EINTR);
+    assert(rc >= 0);
+
     if (pfd.revents & G_IO_HUP) {
         pty_chr_state(chr, 0);
     } else {
-- 
2.5.0

[Qemu-devel] [PULL 3/4] main-loop: suppress warnings under qtest

[Paolo Bonzini]

From: "Michael S. Tsirkin" <mst@redhat.com>

commit 01c22f2cdd4fcf02276ea10f48253850a5fd7259 ("main-loop: Suppress
"I/O thread spun" warnings for qtest") doesn't actually disable the
warning for everyone since some tests don't run under the qtest
accelerator.

Check qtest_driver instead.

Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <1448882964-22433-1-git-send-email-mst@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 main-loop.c   | 2 +-
 stubs/qtest.c | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/main-loop.c b/main-loop.c
index df28670..5877615 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -230,7 +230,7 @@ static int os_host_main_loop_wait(int64_t timeout)
     if (!timeout && (spin_counter > MAX_MAIN_LOOP_SPIN)) {
         static bool notified;
 
-        if (!notified && !qtest_enabled()) {
+        if (!notified && !qtest_driver()) {
             fprintf(stderr,
                     "main-loop: WARNING: I/O thread spun for %d iterations\n",
                     MAX_MAIN_LOOP_SPIN);
diff --git a/stubs/qtest.c b/stubs/qtest.c
index dc17594..4dfde61 100644
--- a/stubs/qtest.c
+++ b/stubs/qtest.c
@@ -12,3 +12,8 @@
 
 /* Needed for qtest_allowed() */
 bool qtest_allowed;
+
+bool qtest_driver(void)
+{
+    return false;
+}
-- 
2.5.0

[Qemu-devel] [PULL 4/4] translate-all: ensure host page mask is always extended with 1's

[Paolo Bonzini]

Anthony reported that >4GB guests on Xen with 32bit QEMU broke after
commit 4ed023c ("Round up RAMBlock sizes to host page sizes", 2015-11-05).

In that patch sizes are masked against qemu_host_page_size/mask which
are uintptr_t, and thus 32bit on a 32bit QEMU, even though the ram space
might be bigger than 4GB on Xen.

Since ram_addr_t is not available on user-mode emulation targets, ensure
that we get a sign extension when masking away the low bits of the address.
Remove the ~10 year old scary comment that the type of these variables
is probably wrong, with another equally scary comment.  The new comment
however does not have "???" in it, which is arguably an improvement.

For completeness use the alignment macros in linux-user and bsd-user
instead of manually doing an &.  linux-user and bsd-user are not affected
by the Xen issue, however.

Reviewed-by: Juan Quintela <quintela@redhat.com>
Reported-by: Anthony PERARD <anthony.perard@citrix.com>
Fixes: 4ed023ce2a39ab5812d33cf4d819def168965a7f
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 bsd-user/elfload.c     | 3 +--
 include/exec/cpu-all.h | 8 +++++---
 linux-user/elfload.c   | 3 +--
 linux-user/mmap.c      | 4 +---
 translate-all.c        | 6 +++---
 translate-common.c     | 2 +-
 6 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c
index 351aab1..59a7bdf 100644
--- a/bsd-user/elfload.c
+++ b/bsd-user/elfload.c
@@ -740,8 +740,7 @@ static void padzero(abi_ulong elf_bss, abi_ulong last_bss)
            size must be known */
         if (qemu_real_host_page_size < qemu_host_page_size) {
             abi_ulong end_addr, end_addr1;
-            end_addr1 = (elf_bss + qemu_real_host_page_size - 1) &
-                ~(qemu_real_host_page_size - 1);
+            end_addr1 = REAL_HOST_PAGE_ALIGN(elf_bss);
             end_addr = HOST_PAGE_ALIGN(elf_bss);
             if (end_addr1 < end_addr) {
                 mmap((void *)g2h(end_addr1), end_addr - end_addr1,
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index f9998b9..83b1781 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -174,11 +174,13 @@ extern unsigned long reserved_va;
 #define TARGET_PAGE_MASK ~(TARGET_PAGE_SIZE - 1)
 #define TARGET_PAGE_ALIGN(addr) (((addr) + TARGET_PAGE_SIZE - 1) & TARGET_PAGE_MASK)
 
-/* ??? These should be the larger of uintptr_t and target_ulong.  */
+/* Using intptr_t ensures that qemu_*_page_mask is sign-extended even
+ * when intptr_t is 32-bit and we are aligning a long long.
+ */
 extern uintptr_t qemu_real_host_page_size;
-extern uintptr_t qemu_real_host_page_mask;
+extern intptr_t qemu_real_host_page_mask;
 extern uintptr_t qemu_host_page_size;
-extern uintptr_t qemu_host_page_mask;
+extern intptr_t qemu_host_page_mask;
 
 #define HOST_PAGE_ALIGN(addr) (((addr) + qemu_host_page_size - 1) & qemu_host_page_mask)
 #define REAL_HOST_PAGE_ALIGN(addr) (((addr) + qemu_real_host_page_size - 1) & \
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index d68f5a1..8b17c0e 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -1478,8 +1478,7 @@ static void zero_bss(abi_ulong elf_bss, abi_ulong last_bss, int prot)
 
     host_start = (uintptr_t) g2h(elf_bss);
     host_end = (uintptr_t) g2h(last_bss);
-    host_map_start = (host_start + qemu_real_host_page_size - 1);
-    host_map_start &= -qemu_real_host_page_size;
+    host_map_start = REAL_HOST_PAGE_ALIGN(host_start);
 
     if (host_map_start < host_end) {
         void *p = mmap((void *)host_map_start, host_end - host_map_start,
diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 5606bcd..7b459d5 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -444,9 +444,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
            /* If so, truncate the file map at eof aligned with 
               the hosts real pagesize. Additional anonymous maps
               will be created beyond EOF.  */
-           len = (sb.st_size - offset);
-           len += qemu_real_host_page_size - 1;
-           len &= ~(qemu_real_host_page_size - 1);
+           len = REAL_HOST_PAGE_ALIGN(sb.st_size - offset);
        }
     }
 
diff --git a/translate-all.c b/translate-all.c
index a940bd2..042a857 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -118,7 +118,7 @@ typedef struct PageDesc {
 #define V_L1_SHIFT (L1_MAP_ADDR_SPACE_BITS - TARGET_PAGE_BITS - V_L1_BITS)
 
 uintptr_t qemu_host_page_size;
-uintptr_t qemu_host_page_mask;
+intptr_t qemu_host_page_mask;
 
 /* The bottom level has pointers to PageDesc */
 static void *l1_map[V_L1_SIZE];
@@ -326,14 +326,14 @@ void page_size_init(void)
     /* NOTE: we can always suppose that qemu_host_page_size >=
        TARGET_PAGE_SIZE */
     qemu_real_host_page_size = getpagesize();
-    qemu_real_host_page_mask = ~(qemu_real_host_page_size - 1);
+    qemu_real_host_page_mask = -(intptr_t)qemu_real_host_page_size;
     if (qemu_host_page_size == 0) {
         qemu_host_page_size = qemu_real_host_page_size;
     }
     if (qemu_host_page_size < TARGET_PAGE_SIZE) {
         qemu_host_page_size = TARGET_PAGE_SIZE;
     }
-    qemu_host_page_mask = ~(qemu_host_page_size - 1);
+    qemu_host_page_mask = -(intptr_t)qemu_host_page_size;
 }
 
 static void page_init(void)
diff --git a/translate-common.c b/translate-common.c
index 619feb4..171222d 100644
--- a/translate-common.c
+++ b/translate-common.c
@@ -21,7 +21,7 @@
 #include "qom/cpu.h"
 
 uintptr_t qemu_real_host_page_size;
-uintptr_t qemu_real_host_page_mask;
+intptr_t qemu_real_host_page_mask;
 
 #ifndef CONFIG_USER_ONLY
 /* mask must never be zero, except for A20 change call */
-- 
2.5.0

Re: [Qemu-devel] [PULL 0/4] QEMU changes for 2.5.0-rc3

[Peter Maydell]

On 2 December 2015 at 15:15, Paolo Bonzini <pbonzini@redhat.com> wrote:
> The following changes since commit 9d7b969ea6d9663a94760c6c131481b366f4d38a:
>
>   Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20151201' into staging (2015-12-02 10:16:53 +0000)
>
> are available in the git repository at:
>
>   git://github.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to 0c2d70c448b7853a91cfa63659aa3cc6630fb9be:
>
>   translate-all: ensure host page mask is always extended with 1's (2015-12-02 13:12:30 +0100)
>
> ----------------------------------------------------------------
> * exec.c use after free
> * Xen 32-on-64 breakage
> * missing EINTR
> * naughty warning under qtest
>
> ----------------------------------------------------------------
> Don Slutz (1):
>       exec: Stop using memory after free
>
> Michael S. Tsirkin (1):
>       main-loop: suppress warnings under qtest
>
> Paolo Bonzini (2):
>       qemu-char: retry g_poll on EINTR
>       translate-all: ensure host page mask is always extended with 1's
>

Applied, thanks.

-- PMM