From 9e0f5b8108e248b78444c9a2ec41a8309825736c Mon Sep 17 00:00:00 2001 From: Jason Wang Date: Thu, 12 Mar 2015 17:50:18 +0800 Subject: [PATCH 1/8] virtio: validate the existence of handle_output before calling it We don't validate the existence of handle_output which may let a buggy guest to trigger a SIGSEV easily. E.g: 1) write 10 to queue_sel to a virtio net device with only 1 queue 2) setup an arbitrary pfn 3) then notify queue 10 Fixing this by validating the existence of handle_output before. Cc: qemu-stable@nongnu.org Cc: Michael S. Tsirkin Signed-off-by: Jason Wang Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Don Koch Reviewed-by: Fam Zheng --- hw/virtio/virtio.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 3c6e430048..17c1260c0d 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -759,8 +759,9 @@ void virtio_queue_set_align(VirtIODevice *vdev, int n, int align) void virtio_queue_notify_vq(VirtQueue *vq) { - if (vq->vring.desc) { + if (vq->vring.desc && vq->handle_output) { VirtIODevice *vdev = vq->vdev; + trace_virtio_queue_notify(vdev, vq - vdev->vq, vq); vq->handle_output(vdev, vq); } From 6c936b74235c2d920790a0ff9feb00b07db239c9 Mon Sep 17 00:00:00 2001 From: Stefan Weil Date: Sat, 14 Mar 2015 11:45:18 +0100 Subject: [PATCH 2/8] virtio: Fix memory leaks reported by Coverity All four leaks are similar, so fix them in one patch. Signed-off-by: Stefan Weil Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/9pfs/virtio-9p-local.c | 28 ++++++++-------------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c index d05c91779f..d66abcdbd8 100644 --- a/hw/9pfs/virtio-9p-local.c +++ b/hw/9pfs/virtio-9p-local.c @@ -488,7 +488,7 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path, int err = -1; int serrno = 0; V9fsString fullname; - char *buffer; + char *buffer = NULL; v9fs_string_init(&fullname); v9fs_string_sprintf(&fullname, "%s/%s", dir_path->data, name); @@ -499,7 +499,6 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path, buffer = rpath(fs_ctx, path); err = mknod(buffer, SM_LOCAL_MODE_BITS|S_IFREG, 0); if (err == -1) { - g_free(buffer); goto out; } err = local_set_xattr(buffer, credp); @@ -512,7 +511,6 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path, buffer = rpath(fs_ctx, path); err = mknod(buffer, SM_LOCAL_MODE_BITS|S_IFREG, 0); if (err == -1) { - g_free(buffer); goto out; } err = local_set_mapped_file_attr(fs_ctx, path, credp); @@ -525,7 +523,6 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path, buffer = rpath(fs_ctx, path); err = mknod(buffer, credp->fc_mode, credp->fc_rdev); if (err == -1) { - g_free(buffer); goto out; } err = local_post_create_passthrough(fs_ctx, path, credp); @@ -539,8 +536,8 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path, err_end: remove(buffer); errno = serrno; - g_free(buffer); out: + g_free(buffer); v9fs_string_free(&fullname); return err; } @@ -552,7 +549,7 @@ static int local_mkdir(FsContext *fs_ctx, V9fsPath *dir_path, int err = -1; int serrno = 0; V9fsString fullname; - char *buffer; + char *buffer = NULL; v9fs_string_init(&fullname); v9fs_string_sprintf(&fullname, "%s/%s", dir_path->data, name); @@ -563,7 +560,6 @@ static int local_mkdir(FsContext *fs_ctx, V9fsPath *dir_path, buffer = rpath(fs_ctx, path); err = mkdir(buffer, SM_LOCAL_DIR_MODE_BITS); if (err == -1) { - g_free(buffer); goto out; } credp->fc_mode = credp->fc_mode|S_IFDIR; @@ -576,7 +572,6 @@ static int local_mkdir(FsContext *fs_ctx, V9fsPath *dir_path, buffer = rpath(fs_ctx, path); err = mkdir(buffer, SM_LOCAL_DIR_MODE_BITS); if (err == -1) { - g_free(buffer); goto out; } credp->fc_mode = credp->fc_mode|S_IFDIR; @@ -590,7 +585,6 @@ static int local_mkdir(FsContext *fs_ctx, V9fsPath *dir_path, buffer = rpath(fs_ctx, path); err = mkdir(buffer, credp->fc_mode); if (err == -1) { - g_free(buffer); goto out; } err = local_post_create_passthrough(fs_ctx, path, credp); @@ -604,8 +598,8 @@ static int local_mkdir(FsContext *fs_ctx, V9fsPath *dir_path, err_end: remove(buffer); errno = serrno; - g_free(buffer); out: + g_free(buffer); v9fs_string_free(&fullname); return err; } @@ -659,7 +653,7 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name, int err = -1; int serrno = 0; V9fsString fullname; - char *buffer; + char *buffer = NULL; /* * Mark all the open to not follow symlinks @@ -675,7 +669,6 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name, buffer = rpath(fs_ctx, path); fd = open(buffer, flags, SM_LOCAL_MODE_BITS); if (fd == -1) { - g_free(buffer); err = fd; goto out; } @@ -690,7 +683,6 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name, buffer = rpath(fs_ctx, path); fd = open(buffer, flags, SM_LOCAL_MODE_BITS); if (fd == -1) { - g_free(buffer); err = fd; goto out; } @@ -706,7 +698,6 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name, buffer = rpath(fs_ctx, path); fd = open(buffer, flags, credp->fc_mode); if (fd == -1) { - g_free(buffer); err = fd; goto out; } @@ -724,8 +715,8 @@ err_end: close(fd); remove(buffer); errno = serrno; - g_free(buffer); out: + g_free(buffer); v9fs_string_free(&fullname); return err; } @@ -738,7 +729,7 @@ static int local_symlink(FsContext *fs_ctx, const char *oldpath, int serrno = 0; char *newpath; V9fsString fullname; - char *buffer; + char *buffer = NULL; v9fs_string_init(&fullname); v9fs_string_sprintf(&fullname, "%s/%s", dir_path->data, name); @@ -751,7 +742,6 @@ static int local_symlink(FsContext *fs_ctx, const char *oldpath, buffer = rpath(fs_ctx, newpath); fd = open(buffer, O_CREAT|O_EXCL|O_RDWR|O_NOFOLLOW, SM_LOCAL_MODE_BITS); if (fd == -1) { - g_free(buffer); err = fd; goto out; } @@ -781,7 +771,6 @@ static int local_symlink(FsContext *fs_ctx, const char *oldpath, buffer = rpath(fs_ctx, newpath); fd = open(buffer, O_CREAT|O_EXCL|O_RDWR|O_NOFOLLOW, SM_LOCAL_MODE_BITS); if (fd == -1) { - g_free(buffer); err = fd; goto out; } @@ -810,7 +799,6 @@ static int local_symlink(FsContext *fs_ctx, const char *oldpath, buffer = rpath(fs_ctx, newpath); err = symlink(oldpath, buffer); if (err) { - g_free(buffer); goto out; } err = lchown(buffer, credp->fc_uid, credp->fc_gid); @@ -831,8 +819,8 @@ static int local_symlink(FsContext *fs_ctx, const char *oldpath, err_end: remove(buffer); errno = serrno; - g_free(buffer); out: + g_free(buffer); v9fs_string_free(&fullname); return err; } From 30b04f8711c5191929af4ed03a779646ced3456e Mon Sep 17 00:00:00 2001 From: Chen Fan Date: Fri, 13 Mar 2015 11:18:03 +0800 Subject: [PATCH 3/8] pcie: correct mistaken register bit for End-End TLP Prefix Blocking from pcie spec 7.8.17, the End-End TLP Prefix Blocking bit local is 15(e.g. 0x8000) in device control 2 register. Signed-off-by: Chen Fan Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/pci/pcie.c | 2 +- include/hw/pci/pcie_regs.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c index 1abbbb192e..1463e65b5d 100644 --- a/hw/pci/pcie.c +++ b/hw/pci/pcie.c @@ -84,7 +84,7 @@ int pcie_cap_init(PCIDevice *dev, uint8_t offset, uint8_t type, uint8_t port) pci_set_long(exp_cap + PCI_EXP_DEVCAP2, PCI_EXP_DEVCAP2_EFF | PCI_EXP_DEVCAP2_EETLPP); - pci_set_word(dev->wmask + pos, PCI_EXP_DEVCTL2_EETLPPB); + pci_set_word(dev->wmask + pos + PCI_EXP_DEVCTL2, PCI_EXP_DEVCTL2_EETLPPB); return pos; } diff --git a/include/hw/pci/pcie_regs.h b/include/hw/pci/pcie_regs.h index 652d9fc58c..848ab1c206 100644 --- a/include/hw/pci/pcie_regs.h +++ b/include/hw/pci/pcie_regs.h @@ -72,7 +72,7 @@ #define PCI_EXP_DEVCAP2_EFF 0x100000 #define PCI_EXP_DEVCAP2_EETLPP 0x200000 -#define PCI_EXP_DEVCTL2_EETLPPB 0x80 +#define PCI_EXP_DEVCTL2_EETLPPB 0x8000 /* ARI */ #define PCI_ARI_VER 1 From 77a3c1d730a14c86f3f5692be9906b206afc3d3e Mon Sep 17 00:00:00 2001 From: Chen Fan Date: Fri, 13 Mar 2015 11:18:04 +0800 Subject: [PATCH 4/8] aer: fix wrong check on expose aer tlp prefix log when specify TLP Prefix log as using pcie_aer_inject_error, the TLP prefix log is always discarded. because the check is incorrect, the End-End TLP Prefix Supported bit (PCI_EXP_DEVCAP2_EETLPP) should be in Device Capabilities 2 Register. Signed-off-by: Chen Fan Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/pci/pcie_aer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c index 5a25c32a3d..c7fad3402c 100644 --- a/hw/pci/pcie_aer.c +++ b/hw/pci/pcie_aer.c @@ -433,7 +433,7 @@ static void pcie_aer_update_log(PCIDevice *dev, const PCIEAERErr *err) } if ((err->flags & PCIE_AER_ERR_TLP_PREFIX_PRESENT) && - (pci_get_long(dev->config + dev->exp.exp_cap + PCI_EXP_DEVCTL2) & + (pci_get_long(dev->config + dev->exp.exp_cap + PCI_EXP_DEVCAP2) & PCI_EXP_DEVCAP2_EETLPP)) { for (i = 0; i < ARRAY_SIZE(err->prefix); ++i) { /* 7.10.12 tlp prefix log register */ From b01738c23da718678469cbe85c0a4a82b19652d4 Mon Sep 17 00:00:00 2001 From: Chen Fan Date: Tue, 10 Mar 2015 09:49:48 +0800 Subject: [PATCH 5/8] pcie_aer: fix typos in pcie_aer_inject_error comment Refer to "PCI Express Base Spec3.0", this comments can't fit the description in spec, so we should fix them. Signed-off-by: Chen Fan Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/pci/pcie_aer.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c index c7fad3402c..9daebc2bd8 100644 --- a/hw/pci/pcie_aer.c +++ b/hw/pci/pcie_aer.c @@ -618,11 +618,11 @@ static bool pcie_aer_inject_uncor_error(PCIEAERInject *inj, bool is_fatal) * non-Function specific error must be recorded in all functions. * It is the responsibility of the caller of this function. * It is also caller's responsibility to determine which function should - * report the rerror. + * report the error. * * 6.2.4 Error Logging - * 6.2.5 Sqeunce of Device Error Signaling and Logging Operations - * table 6-2: Flowchard Showing Sequence of Device Error Signaling and Logging + * 6.2.5 Sequence of Device Error Signaling and Logging Operations + * table 6-2: Flowchart Showing Sequence of Device Error Signaling and Logging * Operations */ int pcie_aer_inject_error(PCIDevice *dev, const PCIEAERErr *err) From 310e91f7d0aaec27f55969597ccbb4e83612695e Mon Sep 17 00:00:00 2001 From: Chen Fan Date: Tue, 10 Mar 2015 09:49:49 +0800 Subject: [PATCH 6/8] aer: fix a wrong init PCI_ERR_COR_STATUS w1cmask type register Error Status Register, so this patch fix a wrong definition for PCI_ERR_COR_STATUS register with w1cmask type. Signed-off-by: Chen Fan Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/pci/pcie_aer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c index 9daebc2bd8..9126058ea6 100644 --- a/hw/pci/pcie_aer.c +++ b/hw/pci/pcie_aer.c @@ -123,7 +123,7 @@ int pcie_aer_init(PCIDevice *dev, uint16_t offset) PCI_ERR_UNC_SUPPORTED); pci_long_test_and_set_mask(dev->w1cmask + offset + PCI_ERR_COR_STATUS, - PCI_ERR_COR_STATUS); + PCI_ERR_COR_SUPPORTED); pci_set_long(dev->config + offset + PCI_ERR_COR_MASK, PCI_ERR_COR_MASK_DEFAULT); From 98a2f30a1b5b8b7b35229a705149f020b4918ab8 Mon Sep 17 00:00:00 2001 From: Chen Fan Date: Tue, 10 Mar 2015 09:52:23 +0800 Subject: [PATCH 7/8] pci: fix several trivial typos in comment Signed-off-by: Chen Fan Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- include/hw/pci/pci.h | 2 +- include/hw/pci/pcie_aer.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index be2d9b8703..b97c2956ec 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -137,7 +137,7 @@ enum { #define PCI_CONFIG_HEADER_SIZE 0x40 /* Size of the standard PCI config space */ #define PCI_CONFIG_SPACE_SIZE 0x100 -/* Size of the standart PCIe config space: 4KB */ +/* Size of the standard PCIe config space: 4KB */ #define PCIE_CONFIG_SPACE_SIZE 0x1000 #define PCI_NUM_PINS 4 /* A-D */ diff --git a/include/hw/pci/pcie_aer.h b/include/hw/pci/pcie_aer.h index bcac80a7b0..2fb83882be 100644 --- a/include/hw/pci/pcie_aer.h +++ b/include/hw/pci/pcie_aer.h @@ -51,7 +51,7 @@ struct PCIEAERLog { PCIEAERErr *log; }; -/* aer error message: error signaling message has only error sevirity and +/* aer error message: error signaling message has only error severity and source id. See 2.2.8.3 error signaling messages */ struct PCIEAERMsg { /* From ce394947a75296fc10f1676932473e92aa8be11a Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Wed, 18 Mar 2015 12:45:53 +0100 Subject: [PATCH 8/8] pcie_aer: fix comment to match pcie spec Code comment says "table 6-2" but in fact it's is not a table, it is "Figure 6-2" on page 479. Cc: Chen Fan Reported-by: Michael Tokarev Signed-off-by: Michael S. Tsirkin --- hw/pci/pcie_aer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c index 9126058ea6..eaa3e6ea94 100644 --- a/hw/pci/pcie_aer.c +++ b/hw/pci/pcie_aer.c @@ -622,8 +622,8 @@ static bool pcie_aer_inject_uncor_error(PCIEAERInject *inj, bool is_fatal) * * 6.2.4 Error Logging * 6.2.5 Sequence of Device Error Signaling and Logging Operations - * table 6-2: Flowchart Showing Sequence of Device Error Signaling and Logging - * Operations + * Figure 6-2: Flowchart Showing Sequence of Device Error Signaling and Logging + * Operations */ int pcie_aer_inject_error(PCIDevice *dev, const PCIEAERErr *err) {