qemu: ioport_read, ioport_write: be defensive about 32-bit addresses

On x86, ioport addresses are 16-bit.  That these functions take 32-bit
arguments is a mistake.  Changing the argument type to 16-bit will
discard the top bits of any erroneous values from elsewhere in qemu.

Also, check just before use that the value is in range.  (This turns
an ill-advised change to MAX_IOPORTS into a possible guest crash
rather than a privilege escalation vulnerability.)

And, in the Xen ioreq processor, clamp incoming ioport addresses to
16-bit values.  Xen will never write >16-bit values but the guest may
have access to the ioreq ring.  We want to defend the rest of the qemu
code from wrong values.

This is XSA-199.

Reported-by: yanghongke <yanghongke@huawei.com>
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
(cherry picked from commit b669e922b37b8957248798a5eb7aa96a666cd3fe)
(cherry picked from commit 095261a9ad5c31b9ed431f8382e8aa223089c85b)
(cherry picked from commit 18858e28bb6bae83ddcf413995b2e68c4c7ae03d)

diff --git a/i386-dm/helper2.c b/i386-dm/helper2.c
index a3fdbe31f22f5e6bb5439253bf7dbfab1145ae70..b907eb42006262b2a16021c9765280fcaa838036 100644 (file)
--- a/i386-dm/helper2.c
+++ b/i386-dm/helper2.c
@@ -378,6 +378,7 @@ static void cpu_ioreq_pio(CPUState *env, ioreq_t *req)
         fprintf(stderr, "PIO: bad size (%u)\n", req->size);
         exit(-1);
     }
+    req->addr &= 0x0ffffU;
 
     if (req->dir == IOREQ_READ) {
         if (!req->data_is_ptr) {

diff --git a/vl.c b/vl.c
index f9c4d7ec24ecb5a569f1cd39139f182558cc53c7..c3c5d630e8b3839deae20e4cbaf56273e93b51c3 100644 (file)
--- a/vl.c
+++ b/vl.c
@@ -52,6 +52,7 @@
 
 #include <xen/hvm/hvm_info_table.h>
 
+#include <assert.h>
 #include <unistd.h>
 #include <fcntl.h>
 #include <signal.h>
@@ -290,26 +291,30 @@ PicState2 *isa_pic;
 static IOPortReadFunc default_ioport_readb, default_ioport_readw, default_ioport_readl;
 static IOPortWriteFunc default_ioport_writeb, default_ioport_writew, default_ioport_writel;
 
-static uint32_t ioport_read(int index, uint32_t address)
+static uint32_t ioport_read(int index, uint16_t address)
 {
     static IOPortReadFunc *default_func[3] = {
         default_ioport_readb,
         default_ioport_readw,
         default_ioport_readl
     };
+    if (address >= MAX_IOPORTS)
+        abort();
     IOPortReadFunc *func = ioport_read_table[index][address];
     if (!func)
         func = default_func[index];
     return func(ioport_opaque[address], address);
 }
 
-static void ioport_write(int index, uint32_t address, uint32_t data)
+static void ioport_write(int index, uint16_t address, uint32_t data)
 {
     static IOPortWriteFunc *default_func[3] = {
         default_ioport_writeb,
         default_ioport_writew,
         default_ioport_writel
     };
+    if (address >= MAX_IOPORTS)
+        abort();
     IOPortWriteFunc *func = ioport_write_table[index][address];
     if (!func)
         func = default_func[index];