Commit 87ad860c authored by Paolo Bonzini's avatar Paolo Bonzini Committed by Kevin Wolf
Browse files

nvme: fix out-of-bounds access to the CMB 



Because the CMB BAR has a min_access_size of 2, if you read the last
byte it will try to memcpy *2* bytes from n->cmbuf, causing an off-by-one
error.  This is CVE-2018-16847.

Another way to fix this might be to register the CMB as a RAM memory
region, which would also be more efficient.  However, that might be a
change for big-endian machines; I didn't think this through and I don't
know how real hardware works.  Add a basic testcase for the CMB in case
somebody does this change later on.

Cc: Keith Busch <keith.busch@intel.com>
Cc: qemu-block@nongnu.org
Reported-by: default avatarLi Qiang <liq3ea@gmail.com>
Reviewed-by: default avatarLi Qiang <liq3ea@gmail.com>
Tested-by: default avatarLi Qiang <liq3ea@gmail.com>
Signed-off-by: default avatarPaolo Bonzini <pbonzini@redhat.com>
Reviewed-by: default avatarPhilippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: default avatarPhilippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
parent 6bf74636

hw/block/nvme.c

+1 −1
Original line number Diff line number Diff line
@@ -1201,7 +1201,7 @@ static const MemoryRegionOps nvme_cmb_ops = { 
    .write = nvme_cmb_write,

    .endianness = DEVICE_LITTLE_ENDIAN,

    .impl = {

        .min_access_size = 2,

        .min_access_size = 1,

        .max_access_size = 8,

    },

};

tests/Makefile.include

+1 −1
Original line number Diff line number Diff line
@@ -730,7 +730,7 @@ tests/test-hmp$(EXESUF): tests/test-hmp.o 
tests/machine-none-test$(EXESUF): tests/machine-none-test.o

tests/drive_del-test$(EXESUF): tests/drive_del-test.o $(libqos-virtio-obj-y)

tests/qdev-monitor-test$(EXESUF): tests/qdev-monitor-test.o $(libqos-pc-obj-y)

tests/nvme-test$(EXESUF): tests/nvme-test.o

tests/nvme-test$(EXESUF): tests/nvme-test.o $(libqos-pc-obj-y)

tests/pvpanic-test$(EXESUF): tests/pvpanic-test.o

tests/i82801b11-test$(EXESUF): tests/i82801b11-test.o

tests/ac97-test$(EXESUF): tests/ac97-test.o

tests/nvme-test.c

+58 −10
Original line number Diff line number Diff line
@@ -8,25 +8,73 @@ 
 */



#include "qemu/osdep.h"

#include "qemu/units.h"

#include "libqtest.h"

#include "libqos/libqos-pc.h"



static QOSState *qnvme_start(const char *extra_opts)

{

    QOSState *qs;

    const char *arch = qtest_get_arch();

    const char *cmd = "-drive id=drv0,if=none,file=null-co://,format=raw "

                      "-device nvme,addr=0x4.0,serial=foo,drive=drv0 %s";



    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {

        qs = qtest_pc_boot(cmd, extra_opts ? : "");

        global_qtest = qs->qts;

        return qs;

    }



    g_printerr("nvme tests are only available on x86\n");

    exit(EXIT_FAILURE);

}



static void qnvme_stop(QOSState *qs)

{

    qtest_shutdown(qs);

}



/* Tests only initialization so far. TODO: Replace with functional tests */

static void nop(void)

{

    QOSState *qs;



    qs = qnvme_start(NULL);

    qnvme_stop(qs);

}



int main(int argc, char **argv)

static void nvmetest_cmb_test(void)

{

    int ret;

    const int cmb_bar_size = 2 * MiB;

    QOSState *qs;

    QPCIDevice *pdev;

    QPCIBar bar;



    g_test_init(&argc, &argv, NULL);

    qtest_add_func("/nvme/nop", nop);

    qs = qnvme_start("-global nvme.cmb_size_mb=2");

    pdev = qpci_device_find(qs->pcibus, QPCI_DEVFN(4,0));

    g_assert(pdev != NULL);



    qpci_device_enable(pdev);

    bar = qpci_iomap(pdev, 2, NULL);



    qpci_io_writel(pdev, bar, 0, 0xccbbaa99);

    g_assert_cmpint(qpci_io_readb(pdev, bar, 0), ==, 0x99);

    g_assert_cmpint(qpci_io_readw(pdev, bar, 0), ==, 0xaa99);



    /* Test partially out-of-bounds accesses.  */

    qpci_io_writel(pdev, bar, cmb_bar_size - 1, 0x44332211);

    g_assert_cmpint(qpci_io_readb(pdev, bar, cmb_bar_size - 1), ==, 0x11);

    g_assert_cmpint(qpci_io_readw(pdev, bar, cmb_bar_size - 1), !=, 0x2211);

    g_assert_cmpint(qpci_io_readl(pdev, bar, cmb_bar_size - 1), !=, 0x44332211);

    g_free(pdev);



    qtest_start("-drive id=drv0,if=none,file=null-co://,format=raw "

                "-device nvme,drive=drv0,serial=foo");

    ret = g_test_run();

    qnvme_stop(qs);

}



    qtest_end();

int main(int argc, char **argv)

{

    g_test_init(&argc, &argv, NULL);

    qtest_add_func("/nvme/nop", nop);

    qtest_add_func("/nvme/cmb_test", nvmetest_cmb_test);



    return ret;

    return g_test_run();

}