[Qemu-devel] [PULL 00/11] RCU, KVM, memory API, crypto, Coverity fixes for 2.4.0-rc1

[Qemu-devel] [PULL 00/11] RCU, KVM, memory API, crypto, Coverity fixes for 2.4.0-rc1

[Paolo Bonzini]

The following changes since commit 6169b60285fe1ff730d840a49527e721bfb30899:

  Update version for v2.4.0-rc0 release (2015-07-09 17:56:56 +0100)

are available in the git repository at:

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

for you to fetch changes up to 4a8775ab71d2186fc1cd585ea80c000409965cde:

  crypto: avoid undefined behavior in nettle calls (2015-07-16 18:54:21 +0200)

----------------------------------------------------------------
* rcu_register_thread fixes.
* MIPS-KVM fixes.
* Coverity fixes.
* Nettle function prototype fixes.
* Memory API refcount fix.

----------------------------------------------------------------
Dr. David Alan Gilbert (1):
      RDMA: Fix error exits

Gonglei (2):
      ppc/spapr_drc: fix memory leak
      arm/xlnx-zynqmp: fix memory leak

James Hogan (2):
      mips/kvm: Fix Big endian 32-bit register access
      mips/kvm: Sign extend registers written to KVM

Paolo Bonzini (4):
      rcu: automatically unregister threads when they exit
      rcu: actually register threads that have RCU read-side critical sections
      rcu: detect missing rcu_register_thread()
      memory: fix refcount leak in memory_region_present

Radim Krčmář (2):
      crypto: fix build with nettle >= 3.0.0
      crypto: avoid undefined behavior in nettle calls

 configure              |  4 +++-
 cpus.c                 |  6 ++++++
 crypto/cipher-nettle.c | 51 ++++++++++++++++++++++++++++++++++++++++++++------
 hw/arm/xlnx-zynqmp.c   |  2 ++
 hw/ppc/spapr_drc.c     |  1 +
 include/qemu/rcu.h     |  4 +++-
 iothread.c             |  3 +++
 memory.c               | 44 +++++++++++++++++++++++++++----------------
 migration/migration.c  |  3 +++
 migration/rdma.c       |  8 ++++----
 target-mips/kvm.c      | 21 +++++++--------------
 tests/rcutorture.c     | 10 ----------
 tests/test-rcu-list.c  |  2 ++
 util/rcu.c             | 49 +++++++++++++++++++++++++++++++++++++++++++-----
 14 files changed, 151 insertions(+), 57 deletions(-)
-- 
2.4.3

[Qemu-devel] [PULL 01/11] mips/kvm: Fix Big endian 32-bit register access

[Paolo Bonzini]

From: James Hogan <james.hogan@imgtec.com>

Fix access to 32-bit registers on big endian targets. The pointer passed
to the kernel must be for the actual 32-bit value, not a temporary
64-bit value, otherwise on big endian systems the kernel will only
interpret the upper half.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Leon Alrae <leon.alrae@imgtec.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: kvm@vger.kernel.org
Cc: qemu-stable@nongnu.org
Message-Id: <1429871214-23514-2-git-send-email-james.hogan@imgtec.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target-mips/kvm.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/target-mips/kvm.c b/target-mips/kvm.c
index bd64a70..85256f3 100644
--- a/target-mips/kvm.c
+++ b/target-mips/kvm.c
@@ -235,10 +235,9 @@ int kvm_mips_set_ipi_interrupt(MIPSCPU *cpu, int irq, int level)
 static inline int kvm_mips_put_one_reg(CPUState *cs, uint64_t reg_id,
                                        int32_t *addr)
 {
-    uint64_t val64 = *addr;
     struct kvm_one_reg cp0reg = {
         .id = reg_id,
-        .addr = (uintptr_t)&val64
+        .addr = (uintptr_t)addr
     };
 
     return kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &cp0reg);
@@ -270,18 +269,12 @@ static inline int kvm_mips_put_one_reg64(CPUState *cs, uint64_t reg_id,
 static inline int kvm_mips_get_one_reg(CPUState *cs, uint64_t reg_id,
                                        int32_t *addr)
 {
-    int ret;
-    uint64_t val64 = 0;
     struct kvm_one_reg cp0reg = {
         .id = reg_id,
-        .addr = (uintptr_t)&val64
+        .addr = (uintptr_t)addr
     };
 
-    ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &cp0reg);
-    if (ret >= 0) {
-        *addr = val64;
-    }
-    return ret;
+    return kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &cp0reg);
 }
 
 static inline int kvm_mips_get_one_ulreg(CPUState *cs, uint64 reg_id,
-- 
2.4.3

[Qemu-devel] [PULL 02/11] mips/kvm: Sign extend registers written to KVM

[Paolo Bonzini]

From: James Hogan <james.hogan@imgtec.com>

In case we're running on a 64-bit host, be sure to sign extend the
general purpose registers and hi/lo/pc before writing them to KVM, so as
to take advantage of MIPS32/MIPS64 compatibility.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Leon Alrae <leon.alrae@imgtec.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: kvm@vger.kernel.org
Cc: qemu-stable@nongnu.org
Message-Id: <1429871214-23514-3-git-send-email-james.hogan@imgtec.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target-mips/kvm.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/target-mips/kvm.c b/target-mips/kvm.c
index 85256f3..d287d42 100644
--- a/target-mips/kvm.c
+++ b/target-mips/kvm.c
@@ -628,12 +628,12 @@ int kvm_arch_put_registers(CPUState *cs, int level)
 
     /* Set the registers based on QEMU's view of things */
     for (i = 0; i < 32; i++) {
-        regs.gpr[i] = env->active_tc.gpr[i];
+        regs.gpr[i] = (int64_t)(target_long)env->active_tc.gpr[i];
     }
 
-    regs.hi = env->active_tc.HI[0];
-    regs.lo = env->active_tc.LO[0];
-    regs.pc = env->active_tc.PC;
+    regs.hi = (int64_t)(target_long)env->active_tc.HI[0];
+    regs.lo = (int64_t)(target_long)env->active_tc.LO[0];
+    regs.pc = (int64_t)(target_long)env->active_tc.PC;
 
     ret = kvm_vcpu_ioctl(cs, KVM_SET_REGS, &regs);
 
-- 
2.4.3

[Qemu-devel] [PULL 03/11] ppc/spapr_drc: fix memory leak

[Paolo Bonzini]

From: Gonglei <arei.gonglei@huawei.com>

fix CID 1311373.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Message-Id: <1436489490-236-3-git-send-email-arei.gonglei@huawei.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/ppc/spapr_drc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index ef98538..ee87432 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -422,6 +422,7 @@ static void realize(DeviceState *d, Error **errp)
         error_free(err);
         object_unref(OBJECT(drc));
     }
+    g_free(child_name);
     DPRINTFN("drc realize complete");
 }
 
-- 
2.4.3

[Qemu-devel] [PULL 04/11] arm/xlnx-zynqmp: fix memory leak

[Paolo Bonzini]

From: Gonglei <arei.gonglei@huawei.com>

fix CID 1311372.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Message-Id: <1436489490-236-4-git-send-email-arei.gonglei@huawei.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/arm/xlnx-zynqmp.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 5e72078..62ef4ce 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -144,6 +144,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
         } else {
             s->boot_cpu_ptr = &s->apu_cpu[i];
         }
+        g_free(name);
 
         object_property_set_int(OBJECT(&s->apu_cpu[i]), GIC_BASE_ADDR,
                                 "reset-cbar", &err);
@@ -181,6 +182,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
         } else {
             s->boot_cpu_ptr = &s->rpu_cpu[i];
         }
+        g_free(name);
 
         object_property_set_bool(OBJECT(&s->rpu_cpu[i]), true, "reset-hivecs",
                                  &err);
-- 
2.4.3

[Qemu-devel] [PULL 05/11] RDMA: Fix error exits

[Paolo Bonzini]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

The error checks I added used 'break' after the error, but I'm
in a switch inside the while loop, so they need to be 'goto out'.

Spotted by coverity; entries 1311368 and 1311369

Fixes: afcddefd

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <1436555332-19076-1-git-send-email-dgilbert@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 migration/rdma.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index f106b2a..74876fd 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -2997,7 +2997,7 @@ static int qemu_rdma_registration_handle(QEMUFile *f, void *opaque)
                              (unsigned int)comp->block_idx,
                              rdma->local_ram_blocks.nb_blocks);
                 ret = -EIO;
-                break;
+                goto out;
             }
             block = &(rdma->local_ram_blocks.block[comp->block_idx]);
 
@@ -3092,7 +3092,7 @@ static int qemu_rdma_registration_handle(QEMUFile *f, void *opaque)
                                  (unsigned int)reg->current_index,
                                  rdma->local_ram_blocks.nb_blocks);
                     ret = -ENOENT;
-                    break;
+                    goto out;
                 }
                 block = &(rdma->local_ram_blocks.block[reg->current_index]);
                 if (block->is_ram_block) {
@@ -3102,7 +3102,7 @@ static int qemu_rdma_registration_handle(QEMUFile *f, void *opaque)
                             block->block_name, block->offset,
                             reg->key.current_addr);
                         ret = -ERANGE;
-                        break;
+                        goto out;
                     }
                     host_addr = (block->local_host_addr +
                                 (reg->key.current_addr - block->offset));
@@ -3118,7 +3118,7 @@ static int qemu_rdma_registration_handle(QEMUFile *f, void *opaque)
                             " chunk: %" PRIx64,
                             block->block_name, reg->key.chunk);
                         ret = -ERANGE;
-                        break;
+                        goto out;
                     }
                 }
                 chunk_start = ram_chunk_start(block, chunk);
-- 
2.4.3

[Qemu-devel] [PULL 06/11] rcu: automatically unregister threads when they exit

[Paolo Bonzini]

This simplifies management within the threads themselves.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/rcutorture.c | 10 ----------
 util/rcu.c         | 12 ++++++++++++
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/tests/rcutorture.c b/tests/rcutorture.c
index d6b304d..000b216 100644
--- a/tests/rcutorture.c
+++ b/tests/rcutorture.c
@@ -134,8 +134,6 @@ static void *rcu_read_perf_test(void *arg)
     qemu_mutex_lock(&counts_mutex);
     n_reads += n_reads_local;
     qemu_mutex_unlock(&counts_mutex);
-
-    rcu_unregister_thread();
     return NULL;
 }
 
@@ -157,8 +155,6 @@ static void *rcu_update_perf_test(void *arg)
     qemu_mutex_lock(&counts_mutex);
     n_updates += n_updates_local;
     qemu_mutex_unlock(&counts_mutex);
-
-    rcu_unregister_thread();
     return NULL;
 }
 
@@ -283,8 +279,6 @@ static void *rcu_read_stress_test(void *arg)
         rcu_stress_count[i] += rcu_stress_local[i];
     }
     qemu_mutex_unlock(&counts_mutex);
-
-    rcu_unregister_thread();
     return NULL;
 }
 
@@ -319,8 +313,6 @@ static void *rcu_update_stress_test(void *arg)
         synchronize_rcu();
         n_updates++;
     }
-
-    rcu_unregister_thread();
     return NULL;
 }
 
@@ -336,8 +328,6 @@ static void *rcu_fake_update_stress_test(void *arg)
         synchronize_rcu();
         g_usleep(1000);
     }
-
-    rcu_unregister_thread();
     return NULL;
 }
 
diff --git a/util/rcu.c b/util/rcu.c
index 7270151..8830295 100644
--- a/util/rcu.c
+++ b/util/rcu.c
@@ -268,12 +268,22 @@ void call_rcu1(struct rcu_head *node, void (*func)(struct rcu_head *node))
     qemu_event_set(&rcu_call_ready_event);
 }
 
+static __thread Notifier unregister_thread_notifier;
+
+static void rcu_unregister_thread_notify(Notifier *n, void *data)
+{
+    rcu_unregister_thread();
+}
+
 void rcu_register_thread(void)
 {
     assert(rcu_reader.ctr == 0);
     qemu_mutex_lock(&rcu_gp_lock);
     QLIST_INSERT_HEAD(&registry, &rcu_reader, node);
     qemu_mutex_unlock(&rcu_gp_lock);
+
+    unregister_thread_notifier.notify = rcu_unregister_thread_notify;
+    qemu_thread_atexit_add(&unregister_thread_notifier);
 }
 
 void rcu_unregister_thread(void)
@@ -281,6 +291,8 @@ void rcu_unregister_thread(void)
     qemu_mutex_lock(&rcu_gp_lock);
     QLIST_REMOVE(&rcu_reader, node);
     qemu_mutex_unlock(&rcu_gp_lock);
+
+    qemu_thread_atexit_remove(&unregister_thread_notifier);
 }
 
 static void rcu_init_complete(void)
-- 
2.4.3

[Qemu-devel] [PULL 07/11] rcu: actually register threads that have RCU read-side critical sections

[Paolo Bonzini]

Otherwise, grace periods are detected too early!

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpus.c                | 6 ++++++
 iothread.c            | 3 +++
 migration/migration.c | 3 +++
 tests/test-rcu-list.c | 2 ++
 util/rcu.c            | 2 ++
 5 files changed, 16 insertions(+)

diff --git a/cpus.c b/cpus.c
index b00a423..a822ce3 100644
--- a/cpus.c
+++ b/cpus.c
@@ -954,6 +954,8 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
     CPUState *cpu = arg;
     int r;
 
+    rcu_register_thread();
+
     qemu_mutex_lock_iothread();
     qemu_thread_get_self(cpu->thread);
     cpu->thread_id = qemu_get_thread_id();
@@ -995,6 +997,8 @@ static void *qemu_dummy_cpu_thread_fn(void *arg)
     sigset_t waitset;
     int r;
 
+    rcu_register_thread();
+
     qemu_mutex_lock_iothread();
     qemu_thread_get_self(cpu->thread);
     cpu->thread_id = qemu_get_thread_id();
@@ -1034,6 +1038,8 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
 {
     CPUState *cpu = arg;
 
+    rcu_register_thread();
+
     qemu_mutex_lock_iothread();
     qemu_tcg_init_cpu_signals();
     qemu_thread_get_self(cpu->thread);
diff --git a/iothread.c b/iothread.c
index 6d2a33f..443d176 100644
--- a/iothread.c
+++ b/iothread.c
@@ -18,6 +18,7 @@
 #include "sysemu/iothread.h"
 #include "qmp-commands.h"
 #include "qemu/error-report.h"
+#include "qemu/rcu.h"
 
 typedef ObjectClass IOThreadClass;
 
@@ -31,6 +32,8 @@ static void *iothread_run(void *opaque)
     IOThread *iothread = opaque;
     bool blocking;
 
+    rcu_register_thread();
+
     qemu_mutex_lock(&iothread->init_done_lock);
     iothread->thread_id = qemu_get_thread_id();
     qemu_cond_signal(&iothread->init_done_cond);
diff --git a/migration/migration.c b/migration/migration.c
index 45719a0..7f1e05a 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -22,6 +22,7 @@
 #include "block/block.h"
 #include "qapi/qmp/qerror.h"
 #include "qemu/sockets.h"
+#include "qemu/rcu.h"
 #include "migration/block.h"
 #include "qemu/thread.h"
 #include "qmp-commands.h"
@@ -911,6 +912,8 @@ static void *migration_thread(void *opaque)
     int64_t start_time = initial_time;
     bool old_vm_running = false;
 
+    rcu_register_thread();
+
     qemu_savevm_state_header(s->file);
     qemu_savevm_state_begin(s->file, &s->params);
 
diff --git a/tests/test-rcu-list.c b/tests/test-rcu-list.c
index 4c5f62e..af98bdb 100644
--- a/tests/test-rcu-list.c
+++ b/tests/test-rcu-list.c
@@ -108,6 +108,8 @@ static void *rcu_q_reader(void *arg)
     long long n_reads_local = 0;
     struct list_element *el;
 
+    rcu_register_thread();
+
     *(struct rcu_reader_data **)arg = &rcu_reader;
     atomic_inc(&nthreadsrunning);
     while (goflag == GOFLAG_INIT) {
diff --git a/util/rcu.c b/util/rcu.c
index 8830295..e21bb46 100644
--- a/util/rcu.c
+++ b/util/rcu.c
@@ -216,6 +216,8 @@ static void *call_rcu_thread(void *opaque)
 {
     struct rcu_head *node;
 
+    rcu_register_thread();
+
     for (;;) {
         int tries = 0;
         int n = atomic_read(&rcu_call_count);
-- 
2.4.3

[Qemu-devel] [PULL 08/11] rcu: detect missing rcu_register_thread()

[Paolo Bonzini]

Use an "impossible" value for the .depth field in order to quickly
detect threads that have not registered themselves with the RCU
subsystem.

Avoid a false positive around forking by unregistering and registering
the forking thread explicitly.  Previously, it was enough to re-register
the thread.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/qemu/rcu.h |  4 +++-
 util/rcu.c         | 35 ++++++++++++++++++++++++++++++-----
 2 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/include/qemu/rcu.h b/include/qemu/rcu.h
index 7df1e86..4facb35 100644
--- a/include/qemu/rcu.h
+++ b/include/qemu/rcu.h
@@ -82,7 +82,9 @@ static inline void rcu_read_lock(void)
     struct rcu_reader_data *p_rcu_reader = &rcu_reader;
     unsigned ctr;
 
-    if (p_rcu_reader->depth++ > 0) {
+    p_rcu_reader->depth++;
+    assert(p_rcu_reader->depth >= 1);
+    if (p_rcu_reader->depth > 1) {
         return;
     }
 
diff --git a/util/rcu.c b/util/rcu.c
index e21bb46..2490273 100644
--- a/util/rcu.c
+++ b/util/rcu.c
@@ -63,8 +63,11 @@ static inline int rcu_gp_ongoing(unsigned long *ctr)
 
 /* Written to only by each individual reader. Read by both the reader and the
  * writers.
+ *
+ * Initializing the depth to -1 causes an assertion failure on the first
+ * call to rcu_read_lock() if the thread does not call rcu_register_thread().
  */
-__thread struct rcu_reader_data rcu_reader;
+__thread struct rcu_reader_data rcu_reader = { .depth = -1 };
 
 /* Protected by rcu_gp_lock.  */
 typedef QLIST_HEAD(, rcu_reader_data) ThreadList;
@@ -279,7 +282,12 @@ static void rcu_unregister_thread_notify(Notifier *n, void *data)
 
 void rcu_register_thread(void)
 {
-    assert(rcu_reader.ctr == 0);
+    /* rcu_reader.depth is also used to detect whether the thread is
+     * registered.
+     */
+    assert(rcu_reader.depth == -1);
+    rcu_reader.depth = 0;
+
     qemu_mutex_lock(&rcu_gp_lock);
     QLIST_INSERT_HEAD(&registry, &rcu_reader, node);
     qemu_mutex_unlock(&rcu_gp_lock);
@@ -290,6 +298,12 @@ void rcu_register_thread(void)
 
 void rcu_unregister_thread(void)
 {
+    /* Resetting the depth to -1 causes an assertion failure on the next
+     * call to rcu_read_lock().
+     */
+    assert(rcu_reader.depth == 0);
+    rcu_reader.depth = -1;
+
     qemu_mutex_lock(&rcu_gp_lock);
     QLIST_REMOVE(&rcu_reader, node);
     qemu_mutex_unlock(&rcu_gp_lock);
@@ -301,7 +315,6 @@ static void rcu_init_complete(void)
 {
     QemuThread thread;
 
-    qemu_mutex_init(&rcu_gp_lock);
     qemu_event_init(&rcu_gp_event, true);
 
     qemu_event_init(&rcu_call_ready_event, false);
@@ -311,8 +324,6 @@ static void rcu_init_complete(void)
      */
     qemu_thread_create(&thread, "call_rcu", call_rcu_thread,
                        NULL, QEMU_THREAD_DETACHED);
-
-    rcu_register_thread();
 }
 
 #ifdef CONFIG_POSIX
@@ -329,14 +340,28 @@ static void rcu_init_unlock(void)
 
 void rcu_after_fork(void)
 {
+    int save_depth = rcu_reader.depth;
+    if (save_depth != -1) {
+        rcu_unregister_thread();
+    }
+
     memset(&registry, 0, sizeof(registry));
+
     rcu_init_complete();
+
+    if (save_depth != -1) {
+        rcu_register_thread();
+        rcu_reader.depth = save_depth;
+    }
 }
 
 static void __attribute__((__constructor__)) rcu_init(void)
 {
+    qemu_mutex_init(&rcu_gp_lock);
 #ifdef CONFIG_POSIX
     pthread_atfork(rcu_init_lock, rcu_init_unlock, rcu_init_unlock);
 #endif
     rcu_init_complete();
+
+    rcu_register_thread();
 }
-- 
2.4.3

Re: [Qemu-devel] [PULL 08/11] rcu: detect missing rcu_register_thread()

[Wen Congyang]

On 07/17/2015 12:55 AM, Paolo Bonzini wrote:
> Use an "impossible" value for the .depth field in order to quickly
> detect threads that have not registered themselves with the RCU
> subsystem.
> 
> Avoid a false positive around forking by unregistering and registering
> the forking thread explicitly.  Previously, it was enough to re-register
> the thread.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  include/qemu/rcu.h |  4 +++-
>  util/rcu.c         | 35 ++++++++++++++++++++++++++++++-----
>  2 files changed, 33 insertions(+), 6 deletions(-)
> 
> diff --git a/include/qemu/rcu.h b/include/qemu/rcu.h
> index 7df1e86..4facb35 100644
> --- a/include/qemu/rcu.h
> +++ b/include/qemu/rcu.h
> @@ -82,7 +82,9 @@ static inline void rcu_read_lock(void)
>      struct rcu_reader_data *p_rcu_reader = &rcu_reader;
>      unsigned ctr;
>  
> -    if (p_rcu_reader->depth++ > 0) {
> +    p_rcu_reader->depth++;
> +    assert(p_rcu_reader->depth >= 1);
> +    if (p_rcu_reader->depth > 1) {
>          return;
>      }
>  
> diff --git a/util/rcu.c b/util/rcu.c
> index e21bb46..2490273 100644
> --- a/util/rcu.c
> +++ b/util/rcu.c
> @@ -63,8 +63,11 @@ static inline int rcu_gp_ongoing(unsigned long *ctr)
>  
>  /* Written to only by each individual reader. Read by both the reader and the
>   * writers.
> + *
> + * Initializing the depth to -1 causes an assertion failure on the first
> + * call to rcu_read_lock() if the thread does not call rcu_register_thread().
>   */
> -__thread struct rcu_reader_data rcu_reader;
> +__thread struct rcu_reader_data rcu_reader = { .depth = -1 };
>  
>  /* Protected by rcu_gp_lock.  */
>  typedef QLIST_HEAD(, rcu_reader_data) ThreadList;
> @@ -279,7 +282,12 @@ static void rcu_unregister_thread_notify(Notifier *n, void *data)
>  
>  void rcu_register_thread(void)
>  {
> -    assert(rcu_reader.ctr == 0);
> +    /* rcu_reader.depth is also used to detect whether the thread is
> +     * registered.
> +     */
> +    assert(rcu_reader.depth == -1);
> +    rcu_reader.depth = 0;
> +
>      qemu_mutex_lock(&rcu_gp_lock);
>      QLIST_INSERT_HEAD(&registry, &rcu_reader, node);
>      qemu_mutex_unlock(&rcu_gp_lock);
> @@ -290,6 +298,12 @@ void rcu_register_thread(void)
>  
>  void rcu_unregister_thread(void)
>  {
> +    /* Resetting the depth to -1 causes an assertion failure on the next
> +     * call to rcu_read_lock().
> +     */
> +    assert(rcu_reader.depth == 0);
> +    rcu_reader.depth = -1;
> +
>      qemu_mutex_lock(&rcu_gp_lock);
>      QLIST_REMOVE(&rcu_reader, node);
>      qemu_mutex_unlock(&rcu_gp_lock);
> @@ -301,7 +315,6 @@ static void rcu_init_complete(void)
>  {
>      QemuThread thread;
>  
> -    qemu_mutex_init(&rcu_gp_lock);
>      qemu_event_init(&rcu_gp_event, true);
>  
>      qemu_event_init(&rcu_call_ready_event, false);
> @@ -311,8 +324,6 @@ static void rcu_init_complete(void)
>       */
>      qemu_thread_create(&thread, "call_rcu", call_rcu_thread,
>                         NULL, QEMU_THREAD_DETACHED);
> -
> -    rcu_register_thread();
>  }
>  
>  #ifdef CONFIG_POSIX
> @@ -329,14 +340,28 @@ static void rcu_init_unlock(void)
>  
>  void rcu_after_fork(void)
>  {
> +    int save_depth = rcu_reader.depth;
> +    if (save_depth != -1) {

If we allow call fork() in RCU read-side critical section,
rcu_reader.depth should be set to 0 before calling
rcu_unregister_thread().

Thanks
Wen Congyang

> +        rcu_unregister_thread();
> +    }
> +
>      memset(&registry, 0, sizeof(registry));
> +
>      rcu_init_complete();
> +
> +    if (save_depth != -1) {
> +        rcu_register_thread();
> +        rcu_reader.depth = save_depth;
> +    }
>  }
>  
>  static void __attribute__((__constructor__)) rcu_init(void)
>  {
> +    qemu_mutex_init(&rcu_gp_lock);
>  #ifdef CONFIG_POSIX
>      pthread_atfork(rcu_init_lock, rcu_init_unlock, rcu_init_unlock);
>  #endif
>      rcu_init_complete();
> +
> +    rcu_register_thread();
>  }
> 

[Qemu-devel] [PULL 09/11] memory: fix refcount leak in memory_region_present

[Paolo Bonzini]

memory_region_present() leaks a reference to a MemoryRegion in the
case "mr == container".  While fixing it, avoid reference counting
altogether for memory_region_present(), by using RCU only.

The return value could in principle be already invalid immediately
after memory_region_present returns, but presumably the caller knows
that and it's using memory_region_present to probe for devices that
are unpluggable, or something like that.  The RCU critical section
is needed anyway, because it protects as->current_map.

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 memory.c | 44 ++++++++++++++++++++++++++++----------------
 1 file changed, 28 insertions(+), 16 deletions(-)

diff --git a/memory.c b/memory.c
index 5a0cc66..0acebb1 100644
--- a/memory.c
+++ b/memory.c
@@ -1887,23 +1887,16 @@ static FlatRange *flatview_lookup(FlatView *view, AddrRange addr)
                    sizeof(FlatRange), cmp_flatrange_addr);
 }
 
-bool memory_region_present(MemoryRegion *container, hwaddr addr)
-{
-    MemoryRegion *mr = memory_region_find(container, addr, 1).mr;
-    if (!mr || (mr == container)) {
-        return false;
-    }
-    memory_region_unref(mr);
-    return true;
-}
-
 bool memory_region_is_mapped(MemoryRegion *mr)
 {
     return mr->container ? true : false;
 }
 
-MemoryRegionSection memory_region_find(MemoryRegion *mr,
-                                       hwaddr addr, uint64_t size)
+/* Same as memory_region_find, but it does not add a reference to the
+ * returned region.  It must be called from an RCU critical section.
+ */
+static MemoryRegionSection memory_region_find_rcu(MemoryRegion *mr,
+                                                  hwaddr addr, uint64_t size)
 {
     MemoryRegionSection ret = { .mr = NULL };
     MemoryRegion *root;
@@ -1924,11 +1917,10 @@ MemoryRegionSection memory_region_find(MemoryRegion *mr,
     }
     range = addrrange_make(int128_make64(addr), int128_make64(size));
 
-    rcu_read_lock();
     view = atomic_rcu_read(&as->current_map);
     fr = flatview_lookup(view, range);
     if (!fr) {
-        goto out;
+        return ret;
     }
 
     while (fr > view->ranges && addrrange_intersects(fr[-1].addr, range)) {
@@ -1944,12 +1936,32 @@ MemoryRegionSection memory_region_find(MemoryRegion *mr,
     ret.size = range.size;
     ret.offset_within_address_space = int128_get64(range.start);
     ret.readonly = fr->readonly;
-    memory_region_ref(ret.mr);
-out:
+    return ret;
+}
+
+MemoryRegionSection memory_region_find(MemoryRegion *mr,
+                                       hwaddr addr, uint64_t size)
+{
+    MemoryRegionSection ret;
+    rcu_read_lock();
+    ret = memory_region_find_rcu(mr, addr, size);
+    if (ret.mr) {
+        memory_region_ref(ret.mr);
+    }
     rcu_read_unlock();
     return ret;
 }
 
+bool memory_region_present(MemoryRegion *container, hwaddr addr)
+{
+    MemoryRegion *mr;
+
+    rcu_read_lock();
+    mr = memory_region_find_rcu(container, addr, 1).mr;
+    rcu_read_unlock();
+    return mr && mr != container;
+}
+
 void address_space_sync_dirty_bitmap(AddressSpace *as)
 {
     FlatView *view;
-- 
2.4.3

[Qemu-devel] [PULL 10/11] crypto: fix build with nettle >= 3.0.0

[Paolo Bonzini]

From: Radim Krčmář <rkrcmar@redhat.com>

In nettle 3, cbc_encrypt() accepts 'nettle_cipher_func' instead of
'nettle_crypt_func' and these two differ in 'const' qualifier of the
first argument.  The build fails with:

  In file included from crypto/cipher.c:71:0:
  ./crypto/cipher-nettle.c: In function ‘qcrypto_cipher_encrypt’:
  ./crypto/cipher-nettle.c:154:38: error: passing argument 2 of
  ‘nettle_cbc_encrypt’ from incompatible pointer type
           cbc_encrypt(ctx->ctx_encrypt, ctx->alg_encrypt,
                                               ^
  In file included from ./crypto/cipher-nettle.c:24:0,
                   from crypto/cipher.c:71:
  /usr/include/nettle/cbc.h:48:1: note: expected
  ‘void (*)(const void *, size_t, uint8_t *, const uint8_t *)
  but argument is of type
  ‘void (*)(      void *, size_t, uint8_t *, const uint8_t *)

To allow both versions, we switch to the new definition and #if typedef
it for old versions.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
Message-Id: <1436548682-9315-2-git-send-email-rkrcmar@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 configure              |  4 +++-
 crypto/cipher-nettle.c | 16 ++++++++++------
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/configure b/configure
index 33b9455..cc0338d 100755
--- a/configure
+++ b/configure
@@ -2183,6 +2183,7 @@ if test "$gnutls_nettle" != "no"; then
     if $pkg_config --exists "nettle"; then
         nettle_cflags=`$pkg_config --cflags nettle`
         nettle_libs=`$pkg_config --libs nettle`
+        nettle_version=`$pkg_config --modversion nettle`
         libs_softmmu="$nettle_libs $libs_softmmu"
         libs_tools="$nettle_libs $libs_tools"
         QEMU_CFLAGS="$QEMU_CFLAGS $nettle_cflags"
@@ -4490,7 +4491,7 @@ echo "GTK support       $gtk"
 echo "GNUTLS support    $gnutls"
 echo "GNUTLS hash       $gnutls_hash"
 echo "GNUTLS gcrypt     $gnutls_gcrypt"
-echo "GNUTLS nettle     $gnutls_nettle"
+echo "GNUTLS nettle     $gnutls_nettle ${gnutls_nettle+($nettle_version)}"
 echo "VTE support       $vte"
 echo "curses support    $curses"
 echo "curl support      $curl"
@@ -4858,6 +4859,7 @@ if test "$gnutls_gcrypt" = "yes" ; then
 fi
 if test "$gnutls_nettle" = "yes" ; then
   echo "CONFIG_GNUTLS_NETTLE=y" >> $config_host_mak
+  echo "CONFIG_NETTLE_VERSION_MAJOR=${nettle_version%%.*}" >> $config_host_mak
 fi
 if test "$vte" = "yes" ; then
   echo "CONFIG_VTE=y" >> $config_host_mak
diff --git a/crypto/cipher-nettle.c b/crypto/cipher-nettle.c
index e5a14bc..e61aaa2 100644
--- a/crypto/cipher-nettle.c
+++ b/crypto/cipher-nettle.c
@@ -23,12 +23,16 @@
 #include <nettle/des.h>
 #include <nettle/cbc.h>
 
+#if CONFIG_NETTLE_VERSION_MAJOR < 3
+typedef nettle_crypt_func nettle_cipher_func;
+#endif
+
 typedef struct QCryptoCipherNettle QCryptoCipherNettle;
 struct QCryptoCipherNettle {
     void *ctx_encrypt;
     void *ctx_decrypt;
-    nettle_crypt_func *alg_encrypt;
-    nettle_crypt_func *alg_decrypt;
+    nettle_cipher_func *alg_encrypt;
+    nettle_cipher_func *alg_decrypt;
     uint8_t *iv;
     size_t niv;
 };
@@ -83,8 +87,8 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
         des_set_key(ctx->ctx_encrypt, rfbkey);
         g_free(rfbkey);
 
-        ctx->alg_encrypt = (nettle_crypt_func *)des_encrypt;
-        ctx->alg_decrypt = (nettle_crypt_func *)des_decrypt;
+        ctx->alg_encrypt = (nettle_cipher_func *)des_encrypt;
+        ctx->alg_decrypt = (nettle_cipher_func *)des_decrypt;
 
         ctx->niv = DES_BLOCK_SIZE;
         break;
@@ -98,8 +102,8 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
         aes_set_encrypt_key(ctx->ctx_encrypt, nkey, key);
         aes_set_decrypt_key(ctx->ctx_decrypt, nkey, key);
 
-        ctx->alg_encrypt = (nettle_crypt_func *)aes_encrypt;
-        ctx->alg_decrypt = (nettle_crypt_func *)aes_decrypt;
+        ctx->alg_encrypt = (nettle_cipher_func *)aes_encrypt;
+        ctx->alg_decrypt = (nettle_cipher_func *)aes_decrypt;
 
         ctx->niv = AES_BLOCK_SIZE;
         break;
-- 
2.4.3

[Qemu-devel] [PULL 11/11] crypto: avoid undefined behavior in nettle calls

[Paolo Bonzini]

From: Radim Krčmář <rkrcmar@redhat.com>

Calling a function pointer that was cast from an incompatible function
results in undefined behavior.  'void *' isn't compatible with 'struct
XXX *', so we can't cast to nettle_cipher_func, but have to provide a
wrapper.  (Conversion from 'void *' to 'struct XXX *' might require
computation, which won't be done if we drop argument's true type, and
pointers can have different sizes so passing arguments on stack would
bug.)

Having two different prototypes based on nettle version doesn't make
this solution any nicer.

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
Message-Id: <1437062641-12684-3-git-send-email-rkrcmar@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 crypto/cipher-nettle.c | 43 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 39 insertions(+), 4 deletions(-)

diff --git a/crypto/cipher-nettle.c b/crypto/cipher-nettle.c
index e61aaa2..a55a8e8 100644
--- a/crypto/cipher-nettle.c
+++ b/crypto/cipher-nettle.c
@@ -25,8 +25,43 @@
 
 #if CONFIG_NETTLE_VERSION_MAJOR < 3
 typedef nettle_crypt_func nettle_cipher_func;
+
+typedef void *       cipher_ctx_t;
+typedef unsigned     cipher_length_t;
+#else
+typedef const void * cipher_ctx_t;
+typedef size_t       cipher_length_t;
 #endif
 
+static nettle_cipher_func aes_encrypt_wrapper;
+static nettle_cipher_func aes_decrypt_wrapper;
+static nettle_cipher_func des_encrypt_wrapper;
+static nettle_cipher_func des_decrypt_wrapper;
+
+static void aes_encrypt_wrapper(cipher_ctx_t ctx, cipher_length_t length,
+                                uint8_t *dst, const uint8_t *src)
+{
+    aes_encrypt(ctx, length, dst, src);
+}
+
+static void aes_decrypt_wrapper(cipher_ctx_t ctx, cipher_length_t length,
+                                uint8_t *dst, const uint8_t *src)
+{
+    aes_encrypt(ctx, length, dst, src);
+}
+
+static void des_encrypt_wrapper(cipher_ctx_t ctx, cipher_length_t length,
+                                uint8_t *dst, const uint8_t *src)
+{
+    des_encrypt(ctx, length, dst, src);
+}
+
+static void des_decrypt_wrapper(cipher_ctx_t ctx, cipher_length_t length,
+                                uint8_t *dst, const uint8_t *src)
+{
+    des_decrypt(ctx, length, dst, src);
+}
+
 typedef struct QCryptoCipherNettle QCryptoCipherNettle;
 struct QCryptoCipherNettle {
     void *ctx_encrypt;
@@ -87,8 +122,8 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
         des_set_key(ctx->ctx_encrypt, rfbkey);
         g_free(rfbkey);
 
-        ctx->alg_encrypt = (nettle_cipher_func *)des_encrypt;
-        ctx->alg_decrypt = (nettle_cipher_func *)des_decrypt;
+        ctx->alg_encrypt = des_encrypt_wrapper;
+        ctx->alg_decrypt = des_decrypt_wrapper;
 
         ctx->niv = DES_BLOCK_SIZE;
         break;
@@ -102,8 +137,8 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
         aes_set_encrypt_key(ctx->ctx_encrypt, nkey, key);
         aes_set_decrypt_key(ctx->ctx_decrypt, nkey, key);
 
-        ctx->alg_encrypt = (nettle_cipher_func *)aes_encrypt;
-        ctx->alg_decrypt = (nettle_cipher_func *)aes_decrypt;
+        ctx->alg_encrypt = aes_encrypt_wrapper;
+        ctx->alg_decrypt = aes_decrypt_wrapper;
 
         ctx->niv = AES_BLOCK_SIZE;
         break;
-- 
2.4.3

Re: [Qemu-devel] [PULL 00/11] RCU, KVM, memory API, crypto, Coverity fixes for 2.4.0-rc1

[Peter Maydell]

On 16 July 2015 at 17:55, Paolo Bonzini <pbonzini@redhat.com> wrote:
> The following changes since commit 6169b60285fe1ff730d840a49527e721bfb30899:
>
>   Update version for v2.4.0-rc0 release (2015-07-09 17:56:56 +0100)
>
> are available in the git repository at:
>
>   git://github.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to 4a8775ab71d2186fc1cd585ea80c000409965cde:
>
>   crypto: avoid undefined behavior in nettle calls (2015-07-16 18:54:21 +0200)
>
> ----------------------------------------------------------------
> * rcu_register_thread fixes.
> * MIPS-KVM fixes.
> * Coverity fixes.
> * Nettle function prototype fixes.
> * Memory API refcount fix.
>

I get a pile of assertions on OSX running rcutorture:

GTESTER tests/rcutorture
Assertion failed: (rcu_reader.depth == 0), function
rcu_unregister_thread, file /Users/pm215/srcAssertion failed:
(rcu_reader.de/qemu/util/rcu.c, line 304.
pth == 0), function rcu_unregister_thread, file
/Users/pm215/src/qemu/util/rcu.c, line 304.
Assertion failed: (rcu_reader.depth == 0), function
rcu_unregister_thread, file /Users/pm215/src/qemu/util/rcu.c, line
304.
Assertion failed: (rcu_reader.depth == 0), function
rcu_unregister_thread, file /Users/pm215/src/qemu/util/rcu.c, line
304.
Assertion failed: (rcu_reader.depth == 0), function
rcu_unregister_thread, file /Users/pm215/src/qemu/util/rcu.c, line
304.
Assertion failed: (rcu_reader.depth == 0), function
rcu_unregister_thread, file /Users/pm215/srcAssertion failed:
(rcu_reader.de/qemu/util/rcu.c, line 304.
pth == 0), function rcu_unregister_thread, file
/Users/pm215/src/qemu/util/rcu.c, line 304.
GTester: last random seed: R02S9b5149dbb406809df60686a3e8223c26
Assertion failed: (rcu_reader.depth == 0), function
rcu_unregister_thread, file /Users/pm215/src/qemu/util/rcu.c, line
304.
Assertion failed: (rcu_reader.depth == 0), function
rcu_unregister_thread, file /Users/pm215/src/qemu/util/rcu.c, line
304.
Assertion failed: (rcu_reader.depth == 0), function
rcu_unregister_thread, file /Users/pm215/src/qemu/util/rcu.c, line
304.
Assertion failed: (rcu_reader.deAssertion failed: (rcu_reader.depth ==
0), function rcu_unregistpth == 0), function rcu_unregister_thread,
file /Users/pm215/srcer_thread, file /Users/pm215/src/qemu/util/rcu.c,
line 304.
/qemu/util/rcu.c, line 304.
GTester: last random seed: R02Sb915fd85eca48d367fd186bdfd39d8c7
make: *** [check-tests/rcutorture] Error 1

-- PMM

Re: [Qemu-devel] [PULL 00/11] RCU, KVM, memory API, crypto, Coverity fixes for 2.4.0-rc1

[Peter Maydell]

On 16 July 2015 at 18:44, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 16 July 2015 at 17:55, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> The following changes since commit 6169b60285fe1ff730d840a49527e721bfb30899:
>>
>>   Update version for v2.4.0-rc0 release (2015-07-09 17:56:56 +0100)
>>
>> are available in the git repository at:
>>
>>   git://github.com/bonzini/qemu.git tags/for-upstream
>>
>> for you to fetch changes up to 4a8775ab71d2186fc1cd585ea80c000409965cde:
>>
>>   crypto: avoid undefined behavior in nettle calls (2015-07-16 18:54:21 +0200)
>>
>> ----------------------------------------------------------------
>> * rcu_register_thread fixes.
>> * MIPS-KVM fixes.
>> * Coverity fixes.
>> * Nettle function prototype fixes.
>> * Memory API refcount fix.
>>
>
> I get a pile of assertions on OSX running rcutorture:

This version of the pull also failed rcutorture on x86-64
linux host, though not with assertions -- looks like it
just exited nonzero.

thanks
-- PMM