Skip to content
Snippets Groups Projects
  1. Mar 23, 2021
  2. Mar 22, 2021
    • Marian Postevca's avatar
      acpi: Move setters/getters of oem fields to X86MachineState · d07b2286
      Marian Postevca authored
      
      The code that sets/gets oem fields is duplicated in both PC and MICROVM
      variants. This commit moves it to X86MachineState so that all x86
      variants can use it and duplication is removed.
      
      Signed-off-by: default avatarMarian Postevca <posteuca@mutex.one>
      Message-Id: <20210221001737.24499-2-posteuca@mutex.one>
      Reviewed-by: default avatarIgor Mammedov <imammedo@redhat.com>
      Reviewed-by: default avatarMichael S. Tsirkin <mst@redhat.com>
      Signed-off-by: default avatarMichael S. Tsirkin <mst@redhat.com>
      d07b2286
    • David Hildenbrand's avatar
      acpi: Set proper maximum size for "etc/acpi/rsdp" blob · 50337286
      David Hildenbrand authored
      
      Let's also set a maximum size for "etc/acpi/rsdp", so the maximum
      size doesn't get implicitly set based on the initial table size. In my
      experiments, the table size was in the range of 22 bytes, so a single
      page (== what we used until now) seems to be good enough.
      
      Now that we have defined maximum sizes for all currently used table types,
      let's assert that we catch usage with new tables that need a proper maximum
      size definition.
      
      Also assert that our initial size does not exceed the maximum size; while
      qemu_ram_alloc_internal() properly asserts that the initial RAMBlock size
      is <= its maximum size, the result might differ when the host page size
      is bigger than 4k.
      
      Suggested-by: default avatarLaszlo Ersek <lersek@redhat.com>
      Cc: Alistair Francis <alistair.francis@xilinx.com>
      Cc: Paolo Bonzini <pbonzini@redhat.com>
      Cc: "Michael S. Tsirkin" <mst@redhat.com>
      Cc: Igor Mammedov <imammedo@redhat.com>
      Cc: Peter Maydell <peter.maydell@linaro.org>
      Cc: Shannon Zhao <shannon.zhaosl@gmail.com>
      Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
      Cc: Paolo Bonzini <pbonzini@redhat.com>
      Cc: Richard Henderson <richard.henderson@linaro.org>
      Cc: Laszlo Ersek <lersek@redhat.com>
      Signed-off-by: default avatarDavid Hildenbrand <david@redhat.com>
      Message-Id: <20210304105554.121674-5-david@redhat.com>
      Reviewed-by: default avatarLaszlo Ersek <lersek@redhat.com>
      Reviewed-by: default avatarIgor Mammedov <imammedo@redhat.com>
      Reviewed-by: default avatarMichael S. Tsirkin <mst@redhat.com>
      Signed-off-by: default avatarMichael S. Tsirkin <mst@redhat.com>
      50337286
    • David Hildenbrand's avatar
      acpi: Move maximum size logic into acpi_add_rom_blob() · 6930ba0d
      David Hildenbrand authored
      
      We want to have safety margins for all tables based on the table type.
      Let's move the maximum size logic into acpi_add_rom_blob() and make it
      dependent on the table name, so we don't have to replicate for each and
      every instance that creates such tables.
      
      Suggested-by: default avatarLaszlo Ersek <lersek@redhat.com>
      Cc: Alistair Francis <alistair.francis@xilinx.com>
      Cc: Paolo Bonzini <pbonzini@redhat.com>
      Cc: "Michael S. Tsirkin" <mst@redhat.com>
      Cc: Igor Mammedov <imammedo@redhat.com>
      Cc: Peter Maydell <peter.maydell@linaro.org>
      Cc: Shannon Zhao <shannon.zhaosl@gmail.com>
      Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
      Cc: Paolo Bonzini <pbonzini@redhat.com>
      Cc: Richard Henderson <richard.henderson@linaro.org>
      Cc: Laszlo Ersek <lersek@redhat.com>
      Signed-off-by: default avatarDavid Hildenbrand <david@redhat.com>
      Message-Id: <20210304105554.121674-4-david@redhat.com>
      Reviewed-by: default avatarLaszlo Ersek <lersek@redhat.com>
      Reviewed-by: default avatarIgor Mammedov <imammedo@redhat.com>
      Reviewed-by: default avatarMichael S. Tsirkin <mst@redhat.com>
      Signed-off-by: default avatarMichael S. Tsirkin <mst@redhat.com>
      6930ba0d
    • David Hildenbrand's avatar
      microvm: Don't open-code "etc/table-loader" · 2a3bdc5c
      David Hildenbrand authored
      
      Let's just reuse ACPI_BUILD_LOADER_FILE.
      
      Cc: Alistair Francis <alistair.francis@xilinx.com>
      Cc: Paolo Bonzini <pbonzini@redhat.com>
      Cc: "Michael S. Tsirkin" <mst@redhat.com>
      Cc: Igor Mammedov <imammedo@redhat.com>
      Cc: Peter Maydell <peter.maydell@linaro.org>
      Cc: Shannon Zhao <shannon.zhaosl@gmail.com>
      Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
      Cc: Paolo Bonzini <pbonzini@redhat.com>
      Cc: Richard Henderson <richard.henderson@linaro.org>
      Cc: Laszlo Ersek <lersek@redhat.com>
      Signed-off-by: default avatarDavid Hildenbrand <david@redhat.com>
      Message-Id: <20210304105554.121674-3-david@redhat.com>
      Reviewed-by: default avatarLaszlo Ersek <lersek@redhat.com>
      Reviewed-by: default avatarIgor Mammedov <imammedo@redhat.com>
      Reviewed-by: default avatarMichael S. Tsirkin <mst@redhat.com>
      Signed-off-by: default avatarMichael S. Tsirkin <mst@redhat.com>
      2a3bdc5c
    • David Hildenbrand's avatar
      acpi: Set proper maximum size for "etc/table-loader" blob · 6c2b24d1
      David Hildenbrand authored
      
      The resizeable memory region / RAMBlock that is created for the cmd blob
      has a maximum size of whole host pages (e.g., 4k), because RAMBlocks
      work on full host pages. In addition, in i386 ACPI code:
        acpi_align_size(tables->linker->cmd_blob, ACPI_BUILD_ALIGN_SIZE);
      makes sure to align to multiples of 4k, padding with 0.
      
      For example, if our cmd_blob is created with a size of 2k, the maximum
      size is 4k - we cannot grow beyond that. Growing might be required
      due to guest action when rebuilding the tables, but also on incoming
      migration.
      
      This automatic generation of the maximum size used to be sufficient,
      however, there are cases where we cross host pages now when growing at
      runtime: we exceed the maximum size of the RAMBlock and can crash QEMU when
      trying to resize the resizeable memory region / RAMBlock:
        $ build/qemu-system-x86_64 --enable-kvm \
            -machine q35,nvdimm=on \
            -smp 1 \
            -cpu host \
            -m size=2G,slots=8,maxmem=4G \
            -object memory-backend-file,id=mem0,mem-path=/tmp/nvdimm,size=256M \
            -device nvdimm,label-size=131072,memdev=mem0,id=nvdimm0,slot=1 \
            -nodefaults \
            -device vmgenid \
            -device intel-iommu
      
      Results in:
        Unexpected error in qemu_ram_resize() at ../softmmu/physmem.c:1850:
        qemu-system-x86_64: Size too large: /rom@etc/table-loader:
          0x2000 > 0x1000: Invalid argument
      
      In this configuration, we consume exactly 4k (32 entries, 128 bytes each)
      when creating the VM. However, once the guest boots up and maps the MCFG,
      we also create the MCFG table and end up consuming 2 additional entries
      (pointer + checksum) -- which is where we try resizing the memory region
      / RAMBlock, however, the maximum size does not allow for it.
      
      Currently, we get the following maximum sizes for our different
      mutable tables based on behavior of resizeable RAMBlock:
      
        hw       table                max_size
        -------  ---------------------------------------------------------
      
        virt     "etc/acpi/tables"    ACPI_BUILD_TABLE_MAX_SIZE (0x200000)
        virt     "etc/table-loader"   HOST_PAGE_ALIGN(initial_size)
        virt     "etc/acpi/rsdp"      HOST_PAGE_ALIGN(initial_size)
      
        i386     "etc/acpi/tables"    ACPI_BUILD_TABLE_MAX_SIZE (0x200000)
        i386     "etc/table-loader"   HOST_PAGE_ALIGN(initial_size)
        i386     "etc/acpi/rsdp"      HOST_PAGE_ALIGN(initial_size)
      
        microvm  "etc/acpi/tables"    ACPI_BUILD_TABLE_MAX_SIZE (0x200000)
        microvm  "etc/table-loader"   HOST_PAGE_ALIGN(initial_size)
        microvm  "etc/acpi/rsdp"      HOST_PAGE_ALIGN(initial_size)
      
      Let's set the maximum table size for "etc/table-loader" to 64k, so we
      can properly grow at runtime, which should be good enough for the future.
      
      Migration is not concerned with the maximum size of a RAMBlock, only
      with the used size - so existing setups are not affected. Of course, we
      cannot migrate a VM that would have crash when started on older QEMU from
      new QEMU to older QEMU without failing early on the destination when
      synchronizing the RAM state:
          qemu-system-x86_64: Size too large: /rom@etc/table-loader: 0x2000 > 0x1000: Invalid argument
          qemu-system-x86_64: error while loading state for instance 0x0 of device 'ram'
          qemu-system-x86_64: load of migration failed: Invalid argument
      
      We'll refactor the code next, to make sure we get rid of this implicit
      behavior for "etc/acpi/rsdp" as well and to make the code easier to
      grasp.
      
      Reviewed-by: default avatarIgor Mammedov <imammedo@redhat.com>
      Cc: Alistair Francis <alistair.francis@xilinx.com>
      Cc: Paolo Bonzini <pbonzini@redhat.com>
      Cc: "Michael S. Tsirkin" <mst@redhat.com>
      Cc: Igor Mammedov <imammedo@redhat.com>
      Cc: Peter Maydell <peter.maydell@linaro.org>
      Cc: Shannon Zhao <shannon.zhaosl@gmail.com>
      Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
      Cc: Paolo Bonzini <pbonzini@redhat.com>
      Cc: Richard Henderson <richard.henderson@linaro.org>
      Cc: Laszlo Ersek <lersek@redhat.com>
      Signed-off-by: default avatarDavid Hildenbrand <david@redhat.com>
      Message-Id: <20210304105554.121674-2-david@redhat.com>
      Reviewed-by: default avatarLaszlo Ersek <lersek@redhat.com>
      Reviewed-by: default avatarIgor Mammedov <imammedo@redhat.com>
      Reviewed-by: default avatarMichael S. Tsirkin <mst@redhat.com>
      Signed-off-by: default avatarMichael S. Tsirkin <mst@redhat.com>
      6c2b24d1
    • Igor Mammedov's avatar
      pci: acpi: add _DSM method to PCI devices · b7f23f62
      Igor Mammedov authored
      
      Implement _DSM according to:
          PCI Firmware Specification 3.1
          4.6.7.  DSM for Naming a PCI or PCI Express Device Under
                  Operating Systems
      and wire it up to cold and hot-plugged PCI devices.
      Feature depends on ACPI hotplug being enabled (as that provides
      PCI devices descriptions in ACPI and MMIO registers that are
      reused to fetch acpi-index).
      
      acpi-index should work for
        - cold plugged NICs:
            $QEMU -device e1000,acpi-index=100
               => 'eno100'
        - hot-plugged
            (monitor) device_add e1000,acpi-index=200,id=remove_me
               => 'eno200'
        - re-plugged
            (monitor) device_del remove_me
            (monitor) device_add e1000,acpi-index=1
               => 'eno1'
      
      Windows also sees index under "PCI Label Id" field in properties
      dialog but otherwise it doesn't seem to have any effect.
      
      Signed-off-by: default avatarIgor Mammedov <imammedo@redhat.com>
      Message-Id: <20210315180102.3008391-6-imammedo@redhat.com>
      Reviewed-by: default avatarMichael S. Tsirkin <mst@redhat.com>
      Signed-off-by: default avatarMichael S. Tsirkin <mst@redhat.com>
      b7f23f62
    • Igor Mammedov's avatar
      acpi: add aml_to_decimalstring() and aml_call6() helpers · 910e4069
      Igor Mammedov authored
      
      it will be used by follow up patches
      
      Signed-off-by: default avatarIgor Mammedov <imammedo@redhat.com>
      Message-Id: <20210315180102.3008391-5-imammedo@redhat.com>
      Reviewed-by: default avatarMichael S. Tsirkin <mst@redhat.com>
      Signed-off-by: default avatarMichael S. Tsirkin <mst@redhat.com>
      910e4069
    • Igor Mammedov's avatar
      pci: acpi: ensure that acpi-index is unique · 4fd7da4c
      Igor Mammedov authored
      
      it helps to avoid device naming conflicts when guest OS is
      configured to use acpi-index for naming.
      Spec ialso says so:
      
      PCI Firmware Specification Revision 3.2
      4.6.7.  _DSM for Naming a PCI or PCI Express Device Under Operating Systems
      "
      Instance number must be unique under \_SB scope. This instance number does not have to
      be sequential in a given system configuration.
      "
      
      Signed-off-by: default avatarIgor Mammedov <imammedo@redhat.com>
      Message-Id: <20210315180102.3008391-4-imammedo@redhat.com>
      Reviewed-by: default avatarMichael S. Tsirkin <mst@redhat.com>
      Signed-off-by: default avatarMichael S. Tsirkin <mst@redhat.com>
      4fd7da4c
    • Igor Mammedov's avatar
      pci: introduce acpi-index property for PCI device · b32bd763
      Igor Mammedov authored
      
      In x86/ACPI world, linux distros are using predictable
      network interface naming since systemd v197. Which on
      QEMU based VMs results into path based naming scheme,
      that names network interfaces based on PCI topology.
      
      With itm on has to plug NIC in exactly the same bus/slot,
      which was used when disk image was first provisioned/configured
      or one risks to loose network configuration due to NIC being
      renamed to actually used topology.
      That also restricts freedom to reshape PCI configuration of
      VM without need to reconfigure used guest image.
      
      systemd also offers "onboard" naming scheme which is
      preferred over PCI slot/topology one, provided that
      firmware implements:
          "
          PCI Firmware Specification 3.1
          4.6.7.  DSM for Naming a PCI or PCI Express Device Under
                  Operating Systems
          "
      that allows to assign user defined index to PCI device,
      which systemd will use to name NIC. For example, using
        -device e1000,acpi-index=100
      guest will rename NIC to 'eno100', where 'eno' is default
      prefix for "onboard" naming scheme. This doesn't require
      any advance configuration on guest side to com in effect
      at 'onboard' scheme takes priority over path based naming.
      
      Hope is that 'acpi-index' it will be easier to consume by
      management layer, compared to forcing specific PCI topology
      and/or having several disk image templates for different
      topologies and will help to simplify process of spawning
      VM from the same template without need to reconfigure
      guest NIC.
      
      This patch adds, 'acpi-index'* property and wires up
      a 32bit register on top of pci hotplug register block
      to pass index value to AML code at runtime.
      Following patch will add corresponding _DSM code and
      wire it up to PCI devices described in ACPI.
      
      *) name comes from linux kernel terminology
      
      Signed-off-by: default avatarIgor Mammedov <imammedo@redhat.com>
      Message-Id: <20210315180102.3008391-3-imammedo@redhat.com>
      Reviewed-by: default avatarMichael S. Tsirkin <mst@redhat.com>
      Signed-off-by: default avatarMichael S. Tsirkin <mst@redhat.com>
      b32bd763
    • Bin Meng's avatar
      hw/sd: sdhci: Reset the data pointer of s->fifo_buffer[] when a different block size is programmed · cffb446e
      Bin Meng authored
      If the block size is programmed to a different value from the
      previous one, reset the data pointer of s->fifo_buffer[] so that
      s->fifo_buffer[] can be filled in using the new block size in
      the next transfer.
      
      With this fix, the following reproducer:
      
      outl 0xcf8 0x80001010
      outl 0xcfc 0xe0000000
      outl 0xcf8 0x80001001
      outl 0xcfc 0x06000000
      write 0xe000002c 0x1 0x05
      write 0xe0000005 0x1 0x02
      write 0xe0000007 0x1 0x01
      write 0xe0000028 0x1 0x10
      write 0x0 0x1 0x23
      write 0x2 0x1 0x08
      write 0xe000000c 0x1 0x01
      write 0xe000000e 0x1 0x20
      write 0xe000000f 0x1 0x00
      write 0xe000000c 0x1 0x32
      write 0xe0000004 0x2 0x0200
      write 0xe0000028 0x1 0x00
      write 0xe0000003 0x1 0x40
      
      cannot be reproduced with the following QEMU command line:
      
      $ qemu-system-x86_64 -nographic -machine accel=qtest -m 512M \
            -nodefaults -device sdhci-pci,sd-spec-version=3 \
            -drive if=sd,index=0,file=null-co://,format=raw,id=mydrive
      
       \
            -device sd-card,drive=mydrive -qtest stdio
      
      Cc: qemu-stable@nongnu.org
      Fixes: CVE-2020-17380
      Fixes: CVE-2020-25085
      Fixes: CVE-2021-3409
      Fixes: d7dfca08 ("hw/sdhci: introduce standard SD host controller")
      Reported-by: default avatarAlexander Bulekov <alxndr@bu.edu>
      Reported-by: Cornelius Aschermann (Ruhr-Universität Bochum)
      Reported-by: Sergej Schumilo (Ruhr-Universität Bochum)
      Reported-by: Simon Wörner (Ruhr-Universität Bochum)
      Buglink: https://bugs.launchpad.net/qemu/+bug/1892960
      Buglink: https://bugs.launchpad.net/qemu/+bug/1909418
      Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1928146
      
      
      Tested-by: default avatarAlexander Bulekov <alxndr@bu.edu>
      Signed-off-by: default avatarBin Meng <bmeng.cn@gmail.com>
      Message-Id: <20210303122639.20004-6-bmeng.cn@gmail.com>
      Signed-off-by: default avatarPhilippe Mathieu-Daudé <f4bug@amsat.org>
      cffb446e
    • Bin Meng's avatar
      hw/sd: sdhci: Limit block size only when SDHC_BLKSIZE register is writable · 5cd7aa34
      Bin Meng authored
      
      The codes to limit the maximum block size is only necessary when
      SDHC_BLKSIZE register is writable.
      
      Tested-by: default avatarAlexander Bulekov <alxndr@bu.edu>
      Reviewed-by: default avatarPhilippe Mathieu-Daudé <f4bug@amsat.org>
      Signed-off-by: default avatarBin Meng <bmeng.cn@gmail.com>
      Message-Id: <20210303122639.20004-5-bmeng.cn@gmail.com>
      Signed-off-by: default avatarPhilippe Mathieu-Daudé <f4bug@amsat.org>
      5cd7aa34
    • Bin Meng's avatar
      hw/sd: sdhci: Correctly set the controller status for ADMA · bc6f2899
      Bin Meng authored
      When an ADMA transfer is started, the codes forget to set the
      controller status to indicate a transfer is in progress.
      
      With this fix, the following 2 reproducers:
      
      https://paste.debian.net/plain/1185136
      https://paste.debian.net/plain/1185141
      
      cannot be reproduced with the following QEMU command line:
      
      $ qemu-system-x86_64 -nographic -machine accel=qtest -m 512M \
            -nodefaults -device sdhci-pci,sd-spec-version=3 \
            -drive if=sd,index=0,file=null-co://,format=raw,id=mydrive
      
       \
            -device sd-card,drive=mydrive -qtest stdio
      
      Cc: qemu-stable@nongnu.org
      Fixes: CVE-2020-17380
      Fixes: CVE-2020-25085
      Fixes: CVE-2021-3409
      Fixes: d7dfca08 ("hw/sdhci: introduce standard SD host controller")
      Reported-by: default avatarAlexander Bulekov <alxndr@bu.edu>
      Reported-by: Cornelius Aschermann (Ruhr-Universität Bochum)
      Reported-by: Sergej Schumilo (Ruhr-Universität Bochum)
      Reported-by: Simon Wörner (Ruhr-Universität Bochum)
      Buglink: https://bugs.launchpad.net/qemu/+bug/1892960
      Buglink: https://bugs.launchpad.net/qemu/+bug/1909418
      Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1928146
      
      
      Tested-by: default avatarAlexander Bulekov <alxndr@bu.edu>
      Reviewed-by: default avatarPhilippe Mathieu-Daudé <f4bug@amsat.org>
      Signed-off-by: default avatarBin Meng <bmeng.cn@gmail.com>
      Message-Id: <20210303122639.20004-4-bmeng.cn@gmail.com>
      Signed-off-by: default avatarPhilippe Mathieu-Daudé <f4bug@amsat.org>
      bc6f2899
    • Bin Meng's avatar
      hw/sd: sdhci: Don't write to SDHC_SYSAD register when transfer is in progress · 8be45cc9
      Bin Meng authored
      Per "SD Host Controller Standard Specification Version 7.00"
      chapter 2.2.1 SDMA System Address Register:
      
      This register can be accessed only if no transaction is executing
      (i.e., after a transaction has stopped).
      
      With this fix, the following reproducer:
      
      outl 0xcf8 0x80001010
      outl 0xcfc 0xfbefff00
      outl 0xcf8 0x80001001
      outl 0xcfc 0x06000000
      write 0xfbefff2c 0x1 0x05
      write 0xfbefff0f 0x1 0x37
      write 0xfbefff0a 0x1 0x01
      write 0xfbefff0f 0x1 0x29
      write 0xfbefff0f 0x1 0x02
      write 0xfbefff0f 0x1 0x03
      write 0xfbefff04 0x1 0x01
      write 0xfbefff05 0x1 0x01
      write 0xfbefff07 0x1 0x02
      write 0xfbefff0c 0x1 0x33
      write 0xfbefff0e 0x1 0x20
      write 0xfbefff0f 0x1 0x00
      write 0xfbefff2a 0x1 0x01
      write 0xfbefff0c 0x1 0x00
      write 0xfbefff03 0x1 0x00
      write 0xfbefff05 0x1 0x00
      write 0xfbefff2a 0x1 0x02
      write 0xfbefff0c 0x1 0x32
      write 0xfbefff01 0x1 0x01
      write 0xfbefff02 0x1 0x01
      write 0xfbefff03 0x1 0x01
      
      cannot be reproduced with the following QEMU command line:
      
      $ qemu-system-x86_64 -nographic -machine accel=qtest -m 512M \
             -nodefaults -device sdhci-pci,sd-spec-version=3 \
             -drive if=sd,index=0,file=null-co://,format=raw,id=mydrive
      
       \
             -device sd-card,drive=mydrive -qtest stdio
      
      Cc: qemu-stable@nongnu.org
      Fixes: CVE-2020-17380
      Fixes: CVE-2020-25085
      Fixes: CVE-2021-3409
      Fixes: d7dfca08 ("hw/sdhci: introduce standard SD host controller")
      Reported-by: default avatarAlexander Bulekov <alxndr@bu.edu>
      Reported-by: Cornelius Aschermann (Ruhr-Universität Bochum)
      Reported-by: Sergej Schumilo (Ruhr-Universität Bochum)
      Reported-by: Simon Wörner (Ruhr-Universität Bochum)
      Buglink: https://bugs.launchpad.net/qemu/+bug/1892960
      Buglink: https://bugs.launchpad.net/qemu/+bug/1909418
      Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1928146
      
      
      Tested-by: default avatarAlexander Bulekov <alxndr@bu.edu>
      Signed-off-by: default avatarBin Meng <bmeng.cn@gmail.com>
      Message-Id: <20210303122639.20004-3-bmeng.cn@gmail.com>
      Signed-off-by: default avatarPhilippe Mathieu-Daudé <f4bug@amsat.org>
      8be45cc9
    • Bin Meng's avatar
      hw/sd: sdhci: Don't transfer any data when command time out · b263d8f9
      Bin Meng authored
      At the end of sdhci_send_command(), it starts a data transfer if the
      command register indicates data is associated. But the data transfer
      should only be initiated when the command execution has succeeded.
      
      With this fix, the following reproducer:
      
      outl 0xcf8 0x80001810
      outl 0xcfc 0xe1068000
      outl 0xcf8 0x80001804
      outw 0xcfc 0x7
      write 0xe106802c 0x1 0x0f
      write 0xe1068004 0xc 0x2801d10101fffffbff28a384
      write 0xe106800c 0x1f 0x9dacbbcad9e8f7061524334251606f7e8d9cabbac9d8e7f60514233241505f
      write 0xe1068003 0x28 0x80d000251480d000252280d000253080d000253e80d000254c80d000255a80d000256880d0002576
      write 0xe1068003 0x1 0xfe
      
      cannot be reproduced with the following QEMU command line:
      
      $ qemu-system-x86_64 -nographic -M pc-q35-5.0 \
            -device sdhci-pci,sd-spec-version=3 \
            -drive if=sd,index=0,file=null-co://,format=raw,id=mydrive
      
       \
            -device sd-card,drive=mydrive \
            -monitor none -serial none -qtest stdio
      
      Cc: qemu-stable@nongnu.org
      Fixes: CVE-2020-17380
      Fixes: CVE-2020-25085
      Fixes: CVE-2021-3409
      Fixes: d7dfca08 ("hw/sdhci: introduce standard SD host controller")
      Reported-by: default avatarAlexander Bulekov <alxndr@bu.edu>
      Reported-by: Cornelius Aschermann (Ruhr-Universität Bochum)
      Reported-by: Sergej Schumilo (Ruhr-Universität Bochum)
      Reported-by: Simon Wörner (Ruhr-Universität Bochum)
      Buglink: https://bugs.launchpad.net/qemu/+bug/1892960
      Buglink: https://bugs.launchpad.net/qemu/+bug/1909418
      Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1928146
      
      
      Acked-by: default avatarAlistair Francis <alistair.francis@wdc.com>
      Tested-by: default avatarAlexander Bulekov <alxndr@bu.edu>
      Tested-by: default avatarPhilippe Mathieu-Daudé <f4bug@amsat.org>
      Signed-off-by: default avatarBin Meng <bmeng.cn@gmail.com>
      Message-Id: <20210303122639.20004-2-bmeng.cn@gmail.com>
      Signed-off-by: default avatarPhilippe Mathieu-Daudé <f4bug@amsat.org>
      b263d8f9
    • Bin Meng's avatar
      hw/sd: sd: Actually perform the erase operation · 818a5cdc
      Bin Meng authored
      
      At present the sd_erase() does not erase the requested range of card
      data to 0xFFs. Let's make the erase operation actually happen.
      
      Signed-off-by: default avatarBin Meng <bin.meng@windriver.com>
      Message-Id: <1613811493-58815-1-git-send-email-bmeng.cn@gmail.com>
      Reviewed-by: default avatarPhilippe Mathieu-Daudé <f4bug@amsat.org>
      Signed-off-by: default avatarPhilippe Mathieu-Daudé <f4bug@amsat.org>
      818a5cdc
    • Bin Meng's avatar
      hw/sd: sd: Fix build error when DEBUG_SD is on · a78d9f27
      Bin Meng authored
      
      "qemu-common.h" should be included to provide the forward declaration
      of qemu_hexdump() when DEBUG_SD is on.
      
      Signed-off-by: default avatarBin Meng <bin.meng@windriver.com>
      Reviewed-by: default avatarPhilippe Mathieu-Daudé <f4bug@amsat.org>
      Message-Id: <20210228050609.24779-1-bmeng.cn@gmail.com>
      Signed-off-by: default avatarPhilippe Mathieu-Daudé <f4bug@amsat.org>
      a78d9f27
    • Wang Liang's avatar
      virtio-pmem: fix virtio_pmem_resp assign problem · d2adda34
      Wang Liang authored
      
      ret in virtio_pmem_resp is a uint32_t variable, which should be assigned
      using virtio_stl_p.
      
      The kernel side driver does not guarantee virtio_pmem_resp to be initialized
      to zero in advance, So sometimes the flush operation will fail.
      
      Signed-off-by: default avatarWang Liang <wangliangzz@inspur.com>
      Message-Id: <20210317024145.271212-1-wangliangzz@126.com>
      Reviewed-by: default avatarStefano Garzarella <sgarzare@redhat.com>
      Reviewed-by: default avatarDavid Hildenbrand <david@redhat.com>
      Reviewed-by: default avatarPankaj Gupta <pankaj.gupta@cloud.ionos.com>
      Reviewed-by: default avatarMichael S. Tsirkin <mst@redhat.com>
      Signed-off-by: default avatarMichael S. Tsirkin <mst@redhat.com>
      d2adda34
    • Greg Kurz's avatar
      vhost-user: Monitor slave channel in vhost_user_read() · db8a3772
      Greg Kurz authored
      
      Now that everything is in place, have the nested event loop to monitor
      the slave channel. The source in the main event loop is destroyed and
      recreated to ensure any pending even for the slave channel that was
      previously detected is purged. This guarantees that the main loop
      wont invoke slave_read() based on an event that was already handled
      by the nested loop.
      
      Signed-off-by: default avatarGreg Kurz <groug@kaod.org>
      Message-Id: <20210312092212.782255-7-groug@kaod.org>
      Reviewed-by: default avatarMichael S. Tsirkin <mst@redhat.com>
      Signed-off-by: default avatarMichael S. Tsirkin <mst@redhat.com>
      Reviewed-by: default avatarStefan Hajnoczi <stefanha@redhat.com>
      db8a3772
    • Greg Kurz's avatar
      vhost-user: Introduce nested event loop in vhost_user_read() · a7f523c7
      Greg Kurz authored
      A deadlock condition potentially exists if a vhost-user process needs
      to request something to QEMU on the slave channel while processing a
      vhost-user message.
      
      This doesn't seem to affect any vhost-user implementation so far, but
      this is currently biting the upcoming enablement of DAX with virtio-fs.
      The issue is being observed when the guest does an emergency reboot while
      a mapping still exits in the DAX window, which is very easy to get with
      a busy enough workload (e.g. as simulated by blogbench [1]) :
      
      - QEMU sends VHOST_USER_GET_VRING_BASE to virtiofsd.
      
      - In order to complete the request, virtiofsd then asks QEMU to remove
        the mapping on the slave channel.
      
      All these dialogs are synchronous, hence the deadlock.
      
      As pointed out by Stefan Hajnoczi:
      
      When QEMU's vhost-user master implementation sends a vhost-user protocol
      message, vhost_user_read() does a "blocking" read during which slave_fd
      is not monitored by QEMU.
      
      The natural solution for this issue is an event loop. The main event
      loop cannot be nested though since we have no guarantees that its
      fd handlers are prepared for re-entrancy.
      
      Introduce a new event loop that only monitors the chardev I/O for now
      in vhost_user_read() and push the actual reading to a one-shot handler.
      A subsequent patch will teach the loop to monitor and process messages
      from the slave channel as well.
      
      [1] https://github.com/jedisct1/Blogbench
      
      
      
      Suggested-by: default avatarStefan Hajnoczi <stefanha@redhat.com>
      Signed-off-by: default avatarGreg Kurz <groug@kaod.org>
      Message-Id: <20210312092212.782255-6-groug@kaod.org>
      Reviewed-by: default avatarMichael S. Tsirkin <mst@redhat.com>
      Signed-off-by: default avatarMichael S. Tsirkin <mst@redhat.com>
      Reviewed-by: default avatarStefan Hajnoczi <stefanha@redhat.com>
      a7f523c7
    • Greg Kurz's avatar
      vhost-user: Convert slave channel to QIOChannelSocket · 57dc0217
      Greg Kurz authored
      
      The slave channel is implemented with socketpair() : QEMU creates
      the pair, passes one of the socket to virtiofsd and monitors the
      other one with the main event loop using qemu_set_fd_handler().
      
      In order to fix a potential deadlock between QEMU and a vhost-user
      external process (e.g. virtiofsd with DAX), we want to be able to
      monitor and service the slave channel while handling vhost-user
      requests.
      
      Prepare ground for this by converting the slave channel to be a
      QIOChannelSocket. This will make monitoring of the slave channel
      as simple as calling qio_channel_add_watch_source(). Since the
      connection is already established between the two sockets, only
      incoming I/O (G_IO_IN) and disconnect (G_IO_HUP) need to be
      serviced.
      
      This also allows to get rid of the ancillary data parsing since
      QIOChannelSocket can do this for us. Note that the MSG_CTRUNC
      check is dropped on the way because QIOChannelSocket ignores this
      case. This isn't a problem since slave_read() provisions space for
      8 file descriptors, but affected vhost-user slave protocol messages
      generally only convey one. If for some reason a buggy implementation
      passes more file descriptors, no need to break the connection, just
      like we don't break it if some other type of ancillary data is
      received : this isn't explicitely violating the protocol per-se so
      it seems better to ignore it.
      
      The current code errors out on short reads and writes. Use the
      qio_channel_*_all() variants to address this on the way.
      
      Signed-off-by: default avatarGreg Kurz <groug@kaod.org>
      Message-Id: <20210312092212.782255-5-groug@kaod.org>
      Reviewed-by: default avatarMichael S. Tsirkin <mst@redhat.com>
      Signed-off-by: default avatarMichael S. Tsirkin <mst@redhat.com>
      Reviewed-by: default avatarStefan Hajnoczi <stefanha@redhat.com>
      57dc0217
    • Greg Kurz's avatar
      vhost-user: Factor out duplicated slave_fd teardown code · de62e494
      Greg Kurz authored
      
      Signed-off-by: default avatarGreg Kurz <groug@kaod.org>
      Message-Id: <20210312092212.782255-4-groug@kaod.org>
      Reviewed-by: default avatarMichael S. Tsirkin <mst@redhat.com>
      Signed-off-by: default avatarMichael S. Tsirkin <mst@redhat.com>
      Reviewed-by: default avatarStefan Hajnoczi <stefanha@redhat.com>
      de62e494
    • Greg Kurz's avatar
      vhost-user: Fix double-close on slave_read() error path · 9e06080b
      Greg Kurz authored
      
      Some message types, e.g. VHOST_USER_SLAVE_VRING_HOST_NOTIFIER_MSG,
      can convey file descriptors. These must be closed before returning
      from slave_read() to avoid being leaked. This can currently be done
      in two different places:
      
      [1] just after the request has been processed
      
      [2] on the error path, under the goto label err:
      
      These path are supposed to be mutually exclusive but they are not
      actually. If the VHOST_USER_NEED_REPLY_MASK flag was passed and the
      sending of the reply fails, both [1] and [2] are performed with the
      same descriptor values. This can potentially cause subtle bugs if one
      of the descriptor was recycled by some other thread in the meantime.
      
      This code duplication complicates rollback for no real good benefit.
      Do the closing in a unique place, under a new fdcleanup: goto label
      at the end of the function.
      
      Signed-off-by: default avatarGreg Kurz <groug@kaod.org>
      Message-Id: <20210312092212.782255-3-groug@kaod.org>
      Reviewed-by: default avatarMichael S. Tsirkin <mst@redhat.com>
      Signed-off-by: default avatarMichael S. Tsirkin <mst@redhat.com>
      Reviewed-by: default avatarStefan Hajnoczi <stefanha@redhat.com>
      9e06080b
    • Greg Kurz's avatar
      vhost-user: Drop misleading EAGAIN checks in slave_read() · a890557d
      Greg Kurz authored
      
      slave_read() checks EAGAIN when reading or writing to the socket
      fails. This gives the impression that the slave channel is in
      non-blocking mode, which is certainly not the case with the current
      code base. And the rest of the code isn't actually ready to cope
      with non-blocking I/O.
      
      Just drop the checks everywhere in this function for the sake of
      clarity.
      
      Signed-off-by: default avatarGreg Kurz <groug@kaod.org>
      Message-Id: <20210312092212.782255-2-groug@kaod.org>
      Reviewed-by: default avatarMichael S. Tsirkin <mst@redhat.com>
      Signed-off-by: default avatarMichael S. Tsirkin <mst@redhat.com>
      Reviewed-by: default avatarStefan Hajnoczi <stefanha@redhat.com>
      a890557d
    • Laurent Vivier's avatar
      virtio: Fix virtio_mmio_read()/virtio_mmio_write() · 0ab8c021
      Laurent Vivier authored
      
      Both functions don't check the personality of the interface (legacy or
      modern) before accessing the configuration memory and always use
      virtio_config_readX()/virtio_config_writeX().
      
      With this patch, they now check the personality and in legacy mode
      call virtio_config_readX()/virtio_config_writeX(), otherwise call
      virtio_config_modern_readX()/virtio_config_modern_writeX().
      
      This change has been tested with virtio-mmio guests (virt stretch/armhf and
      virt sid/m68k) and virtio-pci guests (pseries RHEL-7.3/ppc64 and /ppc64le).
      
      Signed-off-by: default avatarLaurent Vivier <laurent@vivier.eu>
      Message-Id: <20210314200300.3259170-1-laurent@vivier.eu>
      Reviewed-by: default avatarStefano Garzarella <sgarzare@redhat.com>
      Reviewed-by: default avatarMichael S. Tsirkin <mst@redhat.com>
      Signed-off-by: default avatarMichael S. Tsirkin <mst@redhat.com>
      0ab8c021
    • Bin Meng's avatar
      hw/net: virtio-net: Initialize nc->do_not_pad to true · d4c62930
      Bin Meng authored
      
      For virtio-net, there is no need to pad the Ethernet frame size to
      60 bytes before sending to it.
      
      Signed-off-by: default avatarBin Meng <bmeng.cn@gmail.com>
      Signed-off-by: default avatarJason Wang <jasowang@redhat.com>
      d4c62930
  3. Mar 19, 2021
  4. Mar 18, 2021
Loading