hid: clarify hid_keyboard_process_keycode

Coverity thinks the fallthroughs are smelly.  They are correct, but
everything else in this function is like "wut?".

Refer explicitly to bits 8 and 9 of hs->kbd.modifiers instead of
shifting right first and using (1 << 7).  Document what the scancode
is when hid_code is 0xe0.  And add plenty of comments.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

VERSION

@@ -1 +1 @@
-2.3.91
+2.3.90

configure

@@ -2183,7 +2183,6 @@ 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"
@@ -4491,7 +4490,7 @@ echo "GTK support       $gtk"
 echo "GNUTLS support    $gnutls"
 echo "GNUTLS hash       $gnutls_hash"
 echo "GNUTLS gcrypt     $gnutls_gcrypt"
-echo "GNUTLS nettle     $gnutls_nettle ${gnutls_nettle+($nettle_version)}"
+echo "GNUTLS nettle     $gnutls_nettle"
 echo "VTE support       $vte"
 echo "curses support    $curses"
 echo "curl support      $curl"
@@ -4859,7 +4858,6 @@ 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

crypto/cipher-nettle.c

@@ -23,51 +23,12 @@
 #include <nettle/des.h>
 #include <nettle/cbc.h>
 
-#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;
     void *ctx_decrypt;
-    nettle_cipher_func *alg_encrypt;
-    nettle_cipher_func *alg_decrypt;
+    nettle_crypt_func *alg_encrypt;
+    nettle_crypt_func *alg_decrypt;
     uint8_t *iv;
     size_t niv;
 };
@@ -122,8 +83,8 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
         des_set_key(ctx->ctx_encrypt, rfbkey);
         g_free(rfbkey);
 
-        ctx->alg_encrypt = des_encrypt_wrapper;
-        ctx->alg_decrypt = des_decrypt_wrapper;
+        ctx->alg_encrypt = (nettle_crypt_func *)des_encrypt;
+        ctx->alg_decrypt = (nettle_crypt_func *)des_decrypt;
 
         ctx->niv = DES_BLOCK_SIZE;
         break;
@@ -137,8 +98,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 = aes_encrypt_wrapper;
-        ctx->alg_decrypt = aes_decrypt_wrapper;
+        ctx->alg_encrypt = (nettle_crypt_func *)aes_encrypt;
+        ctx->alg_decrypt = (nettle_crypt_func *)aes_decrypt;
 
         ctx->niv = AES_BLOCK_SIZE;
         break;

hw/arm/xlnx-zynqmp.c

@@ -144,7 +144,6 @@ 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);
@@ -182,7 +181,6 @@ 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);

hw/display/qxl.c

@@ -271,12 +271,6 @@ static void qxl_spice_monitors_config_async(PCIQXLDevice *qxl, int replay)
                     QXL_COOKIE_TYPE_POST_LOAD_MONITORS_CONFIG,
                     0));
     } else {
-#if SPICE_SERVER_VERSION >= 0x000c06 /* release 0.12.6 */
-        if (qxl->max_outputs) {
-            spice_qxl_set_monitors_config_limit(&qxl->ssd.qxl,
-                                                qxl->max_outputs);
-        }
-#endif
         qxl->guest_monitors_config = qxl->ram->monitors_config;
         spice_qxl_monitors_config_async(&qxl->ssd.qxl,
                 qxl->ram->monitors_config,
@@ -997,7 +991,6 @@ static int interface_client_monitors_config(QXLInstance *sin,
     PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl);
     QXLRom *rom = memory_region_get_ram_ptr(&qxl->rom_bar);
     int i;
-    unsigned max_outputs = ARRAY_SIZE(rom->client_monitors_config.heads);
 
     if (qxl->revision < 4) {
         trace_qxl_client_monitors_config_unsupported_by_device(qxl->id,
@@ -1020,23 +1013,17 @@ static int interface_client_monitors_config(QXLInstance *sin,
     if (!monitors_config) {
         return 1;
     }
-
-#if SPICE_SERVER_VERSION >= 0x000c06 /* release 0.12.6 */
-    /* limit number of outputs based on setting limit */
-    if (qxl->max_outputs && qxl->max_outputs <= max_outputs) {
-        max_outputs = qxl->max_outputs;
-    }
-#endif
-
     memset(&rom->client_monitors_config, 0,
            sizeof(rom->client_monitors_config));
     rom->client_monitors_config.count = monitors_config->num_of_monitors;
     /* monitors_config->flags ignored */
-    if (rom->client_monitors_config.count >= max_outputs) {
+    if (rom->client_monitors_config.count >=
+            ARRAY_SIZE(rom->client_monitors_config.heads)) {
         trace_qxl_client_monitors_config_capped(qxl->id,
                                 monitors_config->num_of_monitors,
-                                max_outputs);
-        rom->client_monitors_config.count = max_outputs;
+                                ARRAY_SIZE(rom->client_monitors_config.heads));
+        rom->client_monitors_config.count =
+            ARRAY_SIZE(rom->client_monitors_config.heads);
     }
     for (i = 0 ; i < rom->client_monitors_config.count ; ++i) {
         VDAgentMonConfig *monitor = &monitors_config->monitors[i];
@@ -2287,9 +2274,6 @@ static Property qxl_properties[] = {
         DEFINE_PROP_UINT32("vram64_size_mb", PCIQXLDevice, vram_size_mb, -1),
         DEFINE_PROP_UINT32("vgamem_mb", PCIQXLDevice, vgamem_size_mb, 16),
         DEFINE_PROP_INT32("surfaces", PCIQXLDevice, ssd.num_surfaces, 1024),
-#if SPICE_SERVER_VERSION >= 0x000c06 /* release 0.12.6 */
-        DEFINE_PROP_UINT16("max_outputs", PCIQXLDevice, max_outputs, 0),
-#endif
         DEFINE_PROP_END_OF_LIST(),
 };
 

hw/display/qxl.h

@@ -99,9 +99,6 @@ typedef struct PCIQXLDevice {
     QXLModes           *modes;
     uint32_t           rom_size;
     MemoryRegion       rom_bar;
-#if SPICE_SERVER_VERSION >= 0x000c06 /* release 0.12.6 */
-    uint16_t           max_outputs;
-#endif
 
     /* vram pci bar */
     uint32_t           vram_size;

hw/input/hid.c

@@ -239,7 +239,7 @@ static void hid_keyboard_event(DeviceState *dev, QemuConsole *src,
 
 static void hid_keyboard_process_keycode(HIDState *hs)
 {
-    uint8_t hid_code, key;
+    uint8_t hid_code, index, key;
     int i, keycode, slot;
 
     if (hs->n == 0) {
@@ -249,7 +249,8 @@ static void hid_keyboard_process_keycode(HIDState *hs)
     keycode = hs->kbd.keycodes[slot];
 
     key = keycode & 0x7f;
-    hid_code = hid_usage_keys[key | ((hs->kbd.modifiers >> 1) & (1 << 7))];
+    index = key | ((hs->kbd.modifiers & (1 << 8)) >> 1);
+    hid_code = hid_usage_keys[index];
     hs->kbd.modifiers &= ~(1 << 8);
 
     switch (hid_code) {
@@ -257,18 +258,41 @@ static void hid_keyboard_process_keycode(HIDState *hs)
         return;
 
     case 0xe0:
+        assert(key == 0x1d);
         if (hs->kbd.modifiers & (1 << 9)) {
-            hs->kbd.modifiers ^= 3 << 8;
+            /* The hid_codes for the 0xe1/0x1d scancode sequence are 0xe9/0xe0.
+             * Here we're processing the second hid_code.  By dropping bit 9
+             * and setting bit 8, the scancode after 0x1d will access the
+             * second half of the table.
+             */
+            hs->kbd.modifiers ^= (1 << 8) | (1 << 9);
             return;
         }
+        /* fall through to process Ctrl_L */
     case 0xe1 ... 0xe7:
+        /* Ctrl_L/Ctrl_R, Shift_L/Shift_R, Alt_L/Alt_R, Win_L/Win_R.
+         * Handle releases here, or fall through to process presses.
+         */
         if (keycode & (1 << 7)) {
             hs->kbd.modifiers &= ~(1 << (hid_code & 0x0f));
             return;
         }
-    case 0xe8 ... 0xef:
+        /* fall through */
+    case 0xe8 ... 0xe9:
+        /* USB modifiers are just 1 byte long.  Bits 8 and 9 of
+         * hs->kbd.modifiers implement a state machine that detects the
+         * 0xe0 and 0xe1/0x1d sequences.  These bits do not follow the
+         * usual rules where bit 7 marks released keys; they are cleared
+         * elsewhere in the function as the state machine dictates.
+         */
         hs->kbd.modifiers |= 1 << (hid_code & 0x0f);
         return;
+
+    case 0xea ... 0xef:
+        abort();
+
+    default:
+        break;
     }
 
     if (keycode & (1 << 7)) {

hw/input/virtio-input-hid.c

@@ -308,6 +308,7 @@ static void virtio_input_hid_handle_status(VirtIOInput *vinput,
 static Property virtio_input_hid_properties[] = {
     DEFINE_PROP_STRING("display", VirtIOInputHID, display),
     DEFINE_PROP_UINT32("head", VirtIOInputHID, head, 0),
+    DEFINE_PROP_END_OF_LIST(),
 };
 
 static void virtio_input_hid_class_init(ObjectClass *klass, void *data)

hw/input/virtio-input-host.c

@@ -11,6 +11,7 @@
 #include "hw/virtio/virtio.h"
 #include "hw/virtio/virtio-input.h"
 
+#include <sys/ioctl.h>
 #include "standard-headers/linux/input.h"
 
 /* ----------------------------------------------------------------- */

hw/ppc/spapr_drc.c

@@ -422,7 +422,6 @@ static void realize(DeviceState *d, Error **errp)
         error_free(err);
         object_unref(OBJECT(drc));
     }
-    g_free(child_name);
     DPRINTFN("drc realize complete");
 }
 

include/standard-headers/linux/input.h

@@ -10,7 +10,6 @@
 
 
 #include <sys/time.h>
-#include <sys/ioctl.h>
 #include <sys/types.h>
 #include "standard-headers/linux/types.h"
 

memory.c

@@ -1887,16 +1887,23 @@ 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;
 }
 
-/* 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 memory_region_find(MemoryRegion *mr,
+                                       hwaddr addr, uint64_t size)
 {
     MemoryRegionSection ret = { .mr = NULL };
     MemoryRegion *root;
@@ -1917,10 +1924,11 @@ static MemoryRegionSection memory_region_find_rcu(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) {
-        return ret;
+        goto out;
     }
 
     while (fr > view->ranges && addrrange_intersects(fr[-1].addr, range)) {
@@ -1936,32 +1944,12 @@ static MemoryRegionSection memory_region_find_rcu(MemoryRegion *mr,
     ret.size = range.size;
     ret.offset_within_address_space = int128_get64(range.start);
     ret.readonly = fr->readonly;
-    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);
-    }
+    memory_region_ref(ret.mr);
+out:
     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;

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;
-                goto out;
+                break;
             }
             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;
-                    goto out;
+                    break;
                 }
                 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;
-                        goto out;
+                        break;
                     }
                     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;
-                        goto out;
+                        break;
                     }
                 }
                 chunk_start = ram_chunk_start(block, chunk);

scripts/update-linux-headers.sh

@@ -56,6 +56,7 @@ cp_virtio() {
                 -e 's/__bitwise__//' \
                 -e 's/__attribute__((packed))/QEMU_PACKED/' \
                 -e 's/__inline__/inline/' \
+                -e '/sys\/ioctl.h/d' \
                 "$f" > "$to/$header";
         done
     fi

target-mips/kvm.c

@@ -235,9 +235,10 @@ 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)addr
+        .addr = (uintptr_t)&val64
     };
 
     return kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &cp0reg);
@@ -269,12 +270,18 @@ 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)addr
+        .addr = (uintptr_t)&val64
     };
 
-    return kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &cp0reg);
+    ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &cp0reg);
+    if (ret >= 0) {
+        *addr = val64;
+    }
+    return ret;
 }
 
 static inline int kvm_mips_get_one_ulreg(CPUState *cs, uint64 reg_id,
@@ -628,12 +635,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] = (int64_t)(target_long)env->active_tc.gpr[i];
+        regs.gpr[i] = env->active_tc.gpr[i];
     }
 
-    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;
+    regs.hi = env->active_tc.HI[0];
+    regs.lo = env->active_tc.LO[0];
+    regs.pc = env->active_tc.PC;
 
     ret = kvm_vcpu_ioctl(cs, KVM_SET_REGS, &regs);