From 10b5c864793d07cebaeb9d48df6b4066658ca317 Mon Sep 17 00:00:00 2001 Message-Id: <10b5c864793d07cebaeb9d48df6b4066658ca317.1375955382.git.minovotn@redhat.com> In-Reply-To: <7d8ebc793c9bc4b5058ec1189139e7912e209e19.1375955382.git.minovotn@redhat.com> References: <7d8ebc793c9bc4b5058ec1189139e7912e209e19.1375955382.git.minovotn@redhat.com> From: Amos Kong Date: Thu, 8 Aug 2013 04:30:51 +0200 Subject: [PATCH 35/35] virtio: properly validate address before accessing config RH-Author: Amos Kong Message-id: <1375936251-27546-1-git-send-email-akong@redhat.com> Patchwork-id: 53064 O-Subject: [RHEL-6.5 qemu-kvm PATCH] virtio: properly validate address before accessing config Bugzilla: 956953 RH-Acked-by: Michael S. Tsirkin RH-Acked-by: Stefan Hajnoczi RH-Acked-by: Petr Matousek From: Jason Wang Bugzilla: 956953 Brew: http://brewweb.devel.redhat.com/brew/taskinfo?taskID=6143012 Test: tested in localhost There are several several issues in the current checking: - The check was based on the minus of unsigned values which can overflow - It was done after .{set|get}_config() which can lead crash when config_len is zero since vdev->config is NULL Fix this by: - Validate the address in virtio_pci_config_{read|write}() before .{set|get}_config - Use addition instead minus to do the validation Cc: Michael S. Tsirkin Cc: Petr Matousek Signed-off-by: Jason Wang Acked-by: Michael S. Tsirkin Acked-by: Petr Matousek Message-id: 1367905369-10765-1-git-send-email-jasowang@redhat.com Signed-off-by: Anthony Liguori (backport from commit 5f5a1318653c08e435cfa52f60b6a712815b659d) Conflicts: hw/virtio.c Signed-off-by: Amos Kong --- hw/virtio.c | 30 ++++++++++++++++++------------ 1 files changed, 18 insertions(+), 12 deletions(-) Signed-off-by: Michal Novotny --- hw/virtio.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/hw/virtio.c b/hw/virtio.c index 08e64d9..84c717d 100644 --- a/hw/virtio.c +++ b/hw/virtio.c @@ -537,10 +537,11 @@ uint32_t virtio_config_readb(VirtIODevice *vdev, uint32_t addr) { uint8_t val; - vdev->get_config(vdev, vdev->config); - - if (addr > (vdev->config_len - sizeof(val))) + if (addr + sizeof(val) > vdev->config_len) { return (uint32_t)-1; + } + + vdev->get_config(vdev, vdev->config); memcpy(&val, vdev->config + addr, sizeof(val)); return val; @@ -550,10 +551,11 @@ uint32_t virtio_config_readw(VirtIODevice *vdev, uint32_t addr) { uint16_t val; - vdev->get_config(vdev, vdev->config); - - if (addr > (vdev->config_len - sizeof(val))) + if (addr + sizeof(val) > vdev->config_len) { return (uint32_t)-1; + } + + vdev->get_config(vdev, vdev->config); memcpy(&val, vdev->config + addr, sizeof(val)); return val; @@ -563,10 +565,11 @@ uint32_t virtio_config_readl(VirtIODevice *vdev, uint32_t addr) { uint32_t val; - vdev->get_config(vdev, vdev->config); - - if (addr > (vdev->config_len - sizeof(val))) + if (addr + sizeof(val) > vdev->config_len) { return (uint32_t)-1; + } + + vdev->get_config(vdev, vdev->config); memcpy(&val, vdev->config + addr, sizeof(val)); return val; @@ -576,8 +579,9 @@ void virtio_config_writeb(VirtIODevice *vdev, uint32_t addr, uint32_t data) { uint8_t val = data; - if (addr > (vdev->config_len - sizeof(val))) + if (addr + sizeof(val) > vdev->config_len) { return; + } memcpy(vdev->config + addr, &val, sizeof(val)); @@ -589,8 +593,9 @@ void virtio_config_writew(VirtIODevice *vdev, uint32_t addr, uint32_t data) { uint16_t val = data; - if (addr > (vdev->config_len - sizeof(val))) + if (addr + sizeof(val) > vdev->config_len) { return; + } memcpy(vdev->config + addr, &val, sizeof(val)); @@ -602,8 +607,9 @@ void virtio_config_writel(VirtIODevice *vdev, uint32_t addr, uint32_t data) { uint32_t val = data; - if (addr > (vdev->config_len - sizeof(val))) + if (addr + sizeof(val) > vdev->config_len) { return; + } memcpy(vdev->config + addr, &val, sizeof(val)); -- 1.7.11.7