From a0e234d760a899d4e3b8ffaf9215e650cfec747e Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Fri, 5 Feb 2021 10:38:24 +0800 Subject: [PATCH 1/3] ati: use vga_read_byte in ati_cursor_define MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fix CVE-2019-20808 This makes sure reads are confined to vga video memory. v3: use uint32_t, fix cut+paste bug. v2: fix ati_cursor_draw_line too. Reported-by: xu hang Signed-off-by: Gerd Hoffmann Reviewed-by: BALATON Zoltan Reviewed-by: Philippe Mathieu-Daudé Message-id: 20190917111441.27405-3-kraxel@redhat.com cherry-pick from aab0e2a661b2b6bf7915c0aefe807fb60d6d9d13 Signed-off-by: Jiajie Li --- hw/display/ati.c | 21 ++++++++--------- hw/display/vga-access.h | 49 ++++++++++++++++++++++++++++++++++++++++ hw/display/vga-helpers.h | 27 +--------------------- 3 files changed, 60 insertions(+), 37 deletions(-) create mode 100644 hw/display/vga-access.h diff --git a/hw/display/ati.c b/hw/display/ati.c index 5943040416..b17569874e 100644 --- a/hw/display/ati.c +++ b/hw/display/ati.c @@ -19,6 +19,7 @@ #include "qemu/osdep.h" #include "ati_int.h" #include "ati_regs.h" +#include "vga-access.h" #include "vga_regs.h" #include "qemu/log.h" #include "qemu/module.h" @@ -125,20 +126,19 @@ static void ati_vga_switch_mode(ATIVGAState *s) static void ati_cursor_define(ATIVGAState *s) { uint8_t data[1024]; - uint8_t *src; + uint32_t srcoff; int i, j, idx = 0; if ((s->regs.cur_offset & BIT(31)) || s->cursor_guest_mode) { return; /* Do not update cursor if locked or rendered by guest */ } /* FIXME handle cur_hv_offs correctly */ - src = s->vga.vram_ptr + (s->regs.crtc_offset & 0x07ffffff) + - s->regs.cur_offset - (s->regs.cur_hv_offs >> 16) - - (s->regs.cur_hv_offs & 0xffff) * 16; + srcoff = s->regs.cur_offset - + (s->regs.cur_hv_offs >> 16) - (s->regs.cur_hv_offs & 0xffff) * 16; for (i = 0; i < 64; i++) { for (j = 0; j < 8; j++, idx++) { - data[idx] = src[i * 16 + j]; - data[512 + idx] = src[i * 16 + j + 8]; + data[idx] = vga_read_byte(&s->vga, srcoff + i * 16 + j); + data[512 + idx] = vga_read_byte(&s->vga, srcoff + i * 16 + j + 8); } } if (!s->cursor) { @@ -180,7 +180,7 @@ static void ati_cursor_invalidate(VGACommonState *vga) static void ati_cursor_draw_line(VGACommonState *vga, uint8_t *d, int scr_y) { ATIVGAState *s = container_of(vga, ATIVGAState, vga); - uint8_t *src; + uint32_t srcoff; uint32_t *dp = (uint32_t *)d; int i, j, h; @@ -190,14 +190,13 @@ static void ati_cursor_draw_line(VGACommonState *vga, uint8_t *d, int scr_y) return; } /* FIXME handle cur_hv_offs correctly */ - src = s->vga.vram_ptr + (s->regs.crtc_offset & 0x07ffffff) + - s->cursor_offset + (scr_y - vga->hw_cursor_y) * 16; + srcoff = s->cursor_offset + (scr_y - vga->hw_cursor_y) * 16; dp = &dp[vga->hw_cursor_x]; h = ((s->regs.crtc_h_total_disp >> 16) + 1) * 8; for (i = 0; i < 8; i++) { uint32_t color; - uint8_t abits = src[i]; - uint8_t xbits = src[i + 8]; + uint8_t abits = vga_read_byte(vga, srcoff + i); + uint8_t xbits = vga_read_byte(vga, srcoff + i + 8); for (j = 0; j < 8; j++, abits <<= 1, xbits <<= 1) { if (abits & BIT(7)) { if (xbits & BIT(7)) { diff --git a/hw/display/vga-access.h b/hw/display/vga-access.h new file mode 100644 index 0000000000..c0fbd9958b --- /dev/null +++ b/hw/display/vga-access.h @@ -0,0 +1,49 @@ +/* + * QEMU VGA Emulator templates + * + * Copyright (c) 2003 Fabrice Bellard + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +static inline uint8_t vga_read_byte(VGACommonState *vga, uint32_t addr) +{ + return vga->vram_ptr[addr & vga->vbe_size_mask]; +} + +static inline uint16_t vga_read_word_le(VGACommonState *vga, uint32_t addr) +{ + uint32_t offset = addr & vga->vbe_size_mask & ~1; + uint16_t *ptr = (uint16_t *)(vga->vram_ptr + offset); + return lduw_le_p(ptr); +} + +static inline uint16_t vga_read_word_be(VGACommonState *vga, uint32_t addr) +{ + uint32_t offset = addr & vga->vbe_size_mask & ~1; + uint16_t *ptr = (uint16_t *)(vga->vram_ptr + offset); + return lduw_be_p(ptr); +} + +static inline uint32_t vga_read_dword_le(VGACommonState *vga, uint32_t addr) +{ + uint32_t offset = addr & vga->vbe_size_mask & ~3; + uint32_t *ptr = (uint32_t *)(vga->vram_ptr + offset); + return ldl_le_p(ptr); +} diff --git a/hw/display/vga-helpers.h b/hw/display/vga-helpers.h index 5a752b3f9e..5b6c02faa6 100644 --- a/hw/display/vga-helpers.h +++ b/hw/display/vga-helpers.h @@ -21,6 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN * THE SOFTWARE. */ +#include "vga-access.h" static inline void vga_draw_glyph_line(uint8_t *d, uint32_t font_data, uint32_t xorcol, uint32_t bgcol) @@ -95,32 +96,6 @@ static void vga_draw_glyph9(uint8_t *d, int linesize, } while (--h); } -static inline uint8_t vga_read_byte(VGACommonState *vga, uint32_t addr) -{ - return vga->vram_ptr[addr & vga->vbe_size_mask]; -} - -static inline uint16_t vga_read_word_le(VGACommonState *vga, uint32_t addr) -{ - uint32_t offset = addr & vga->vbe_size_mask & ~1; - uint16_t *ptr = (uint16_t *)(vga->vram_ptr + offset); - return lduw_le_p(ptr); -} - -static inline uint16_t vga_read_word_be(VGACommonState *vga, uint32_t addr) -{ - uint32_t offset = addr & vga->vbe_size_mask & ~1; - uint16_t *ptr = (uint16_t *)(vga->vram_ptr + offset); - return lduw_be_p(ptr); -} - -static inline uint32_t vga_read_dword_le(VGACommonState *vga, uint32_t addr) -{ - uint32_t offset = addr & vga->vbe_size_mask & ~3; - uint32_t *ptr = (uint32_t *)(vga->vram_ptr + offset); - return ldl_le_p(ptr); -} - /* * 4 color mode */ -- Gitee From 099f8f56b80edab8561abf83d96b0223efa2257e Mon Sep 17 00:00:00 2001 From: Prasad J Pandit Date: Mon, 8 Feb 2021 17:14:21 +0800 Subject: [PATCH 2/3] sd: sdhci: assert data_count is within fifo_buffer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix CVE-2020-17380 While doing multi block SDMA, transfer block size may exceed the 's->fifo_buffer[s->buf_maxsz]' size. It may leave the current element pointer 's->data_count' pointing out of bounds. Leading the subsequent DMA r/w operation to OOB access issue. Assert that 's->data_count' is within fifo_buffer. -> https://ruhr-uni-bochum.sciebo.de/s/NNWP2GfwzYKeKwE?path=%2Fsdhci_oob_write1 ==1459837==ERROR: AddressSanitizer: heap-buffer-overflow WRITE of size 54722048 at 0x61500001e280 thread T3 #0 __interceptor_memcpy (/lib64/libasan.so.6+0x3a71d) #1 flatview_read_continue ../exec.c:3245 #2 flatview_read ../exec.c:3278 #3 address_space_read_full ../exec.c:3291 #4 address_space_rw ../exec.c:3319 #5 dma_memory_rw_relaxed ../include/sysemu/dma.h:87 #6 dma_memory_rw ../include/sysemu/dma.h:110 #7 dma_memory_read ../include/sysemu/dma.h:116 #8 sdhci_sdma_transfer_multi_blocks ../hw/sd/sdhci.c:629 #9 sdhci_write ../hw/sd/sdhci.c:1097 #10 memory_region_write_accessor ../softmmu/memory.c:483 ... Reported-by: Ruhr-University Suggested-by: Philippe Mathieu-Daudé Signed-off-by: Prasad J Pandit patch link: https://lists.nongnu.org/archive/html/qemu-devel/2020-09/msg01175.html Signed-off-by: Jiajie Li --- hw/sd/sdhci.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c index 65a530aee4..d4ee6bd01f 100644 --- a/hw/sd/sdhci.c +++ b/hw/sd/sdhci.c @@ -613,6 +613,7 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s) s->blkcnt--; } } + assert(s->data_count <= s->buf_maxsz && s->data_count > begin); dma_memory_write(s->dma_as, s->sdmasysad, &s->fifo_buffer[begin], s->data_count - begin); s->sdmasysad += s->data_count - begin; @@ -635,6 +636,7 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s) s->data_count = block_size; boundary_count -= block_size - begin; } + assert(s->data_count <= s->buf_maxsz && s->data_count > begin); dma_memory_read(s->dma_as, s->sdmasysad, &s->fifo_buffer[begin], s->data_count - begin); s->sdmasysad += s->data_count - begin; -- Gitee From 03ba94d1dc5a28ffbdab791c41e9c634407f378f Mon Sep 17 00:00:00 2001 From: Prasad J Pandit Date: Fri, 19 Feb 2021 16:28:00 +0800 Subject: [PATCH 3/3] msix: add valid.accepts methods to check address Fix CVE-2020-13754 While doing msi-x mmio operations, a guest may send an address that leads to an OOB access issue. Add valid.accepts methods to ensure that ensuing mmio r/w operation don't go beyond regions. Reported-by: Ren Ding Reported-by: Hanqing Zhao Reported-by: Anatoly Trosinenko Reported-by: Alexander Bulekov Signed-off-by: Prasad J Pandit patch link: https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg00004.html Signed-off-by: Jiajie Li --- hw/pci/msix.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/hw/pci/msix.c b/hw/pci/msix.c index d39dcf32e8..ec43f16875 100644 --- a/hw/pci/msix.c +++ b/hw/pci/msix.c @@ -192,6 +192,15 @@ static void msix_table_mmio_write(void *opaque, hwaddr addr, msix_handle_mask_update(dev, vector, was_masked); } +static bool msix_table_accepts(void *opaque, hwaddr addr, unsigned size, + bool is_write, MemTxAttrs attrs) +{ + PCIDevice *dev = opaque; + uint16_t tbl_size = dev->msix_entries_nr * PCI_MSIX_ENTRY_SIZE; + + return dev->msix_table + addr + 4 <= dev->msix_table + tbl_size; +} + static const MemoryRegionOps msix_table_mmio_ops = { .read = msix_table_mmio_read, .write = msix_table_mmio_write, @@ -199,6 +208,7 @@ static const MemoryRegionOps msix_table_mmio_ops = { .valid = { .min_access_size = 4, .max_access_size = 4, + .accepts = msix_table_accepts }, }; @@ -220,6 +230,15 @@ static void msix_pba_mmio_write(void *opaque, hwaddr addr, { } +static bool msix_pba_accepts(void *opaque, hwaddr addr, unsigned size, + bool is_write, MemTxAttrs attrs) +{ + PCIDevice *dev = opaque; + uint16_t pba_size = QEMU_ALIGN_UP(dev->msix_entries_nr, 64) / 8; + + return dev->msix_pba + addr + 4 <= dev->msix_pba + pba_size; +} + static const MemoryRegionOps msix_pba_mmio_ops = { .read = msix_pba_mmio_read, .write = msix_pba_mmio_write, @@ -227,6 +246,7 @@ static const MemoryRegionOps msix_pba_mmio_ops = { .valid = { .min_access_size = 4, .max_access_size = 4, + .accepts = msix_pba_accepts }, }; -- Gitee