| Message ID | 20240630130426.2966539-1-zheyuma97@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | hw/display/tcx: Fix out-of-bounds access in tcx_blit_writel | expand |
Hi, zheyu > -----Original Message----- > From: qemu-devel-bounces+yaoxt.fnst=fujitsu.com@nongnu.org > <qemu-devel-bounces+yaoxt.fnst=fujitsu.com@nongnu.org> On Behalf Of Zheyu > Ma > Sent: Sunday, June 30, 2024 9:04 PM > To: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > Cc: Zheyu Ma <zheyuma97@gmail.com>; qemu-devel@nongnu.org > Subject: [PATCH] hw/display/tcx: Fix out-of-bounds access in tcx_blit_writel > > This patch addresses a potential out-of-bounds memory access issue in the > tcx_blit_writel function. It adds bounds checking to ensure that memory > accesses do not exceed the allocated VRAM size. If an out-of-bounds access > is detected, an error is logged using qemu_log_mask. > > ASAN log: > ==2960379==ERROR: AddressSanitizer: SEGV on unknown address > 0x7f524752fd01 (pc 0x7f525c2c4881 bp 0x7ffdaf87bfd0 sp 0x7ffdaf87b788 T0) > ==2960379==The signal is caused by a READ memory access. > #0 0x7f525c2c4881 in memcpy > string/../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:222 > #1 0x55aa782bd5b1 in __asan_memcpy > llvm/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp:22:3 > #2 0x55aa7854dedd in tcx_blit_writel hw/display/tcx.c:590:13 > > Reproducer: > cat << EOF | qemu-system-sparc -display none \ > -machine accel=qtest, -m 512M -machine LX -m 256 -qtest stdio > writel 0x562e98c4 0x3d92fd01 > EOF > > Signed-off-by: Zheyu Ma <zheyuma97@gmail.com> > --- > hw/display/tcx.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/hw/display/tcx.c b/hw/display/tcx.c > index 99507e7638..af43bea7f2 100644 > --- a/hw/display/tcx.c > +++ b/hw/display/tcx.c > @@ -33,6 +33,7 @@ > #include "migration/vmstate.h" > #include "qemu/error-report.h" > #include "qemu/module.h" > +#include "qemu/log.h" > #include "qom/object.h" > > #define TCX_ROM_FILE "QEMU,tcx.bin" > @@ -577,6 +578,14 @@ static void tcx_blit_writel(void *opaque, hwaddr addr, > addr = (addr >> 3) & 0xfffff; > adsr = val & 0xffffff; > len = ((val >> 24) & 0x1f) + 1; > + > + if (addr + len > s->vram_size || adsr + len > s->vram_size) { if adsr == 0xffffff, this condition may be always true, thus the branch “if (adsr == 0xffffff) {” will be never reached. > + qemu_log_mask(LOG_GUEST_ERROR, > + "%s: VRAM access out of bounds. addr: 0x%lx, adsr: > 0x%x, len: %u\n", > + __func__, addr, adsr, len); > + return; > + } > + > if (adsr == 0xffffff) { > memset(&s->vram[addr], s->tmpblit, len); > if (s->depth == 24) { > -- > 2.34.1 >
On 30/06/2024 14:04, Zheyu Ma wrote: > This patch addresses a potential out-of-bounds memory access issue in the > tcx_blit_writel function. It adds bounds checking to ensure that memory > accesses do not exceed the allocated VRAM size. If an out-of-bounds access > is detected, an error is logged using qemu_log_mask. > > ASAN log: > ==2960379==ERROR: AddressSanitizer: SEGV on unknown address 0x7f524752fd01 (pc 0x7f525c2c4881 bp 0x7ffdaf87bfd0 sp 0x7ffdaf87b788 T0) > ==2960379==The signal is caused by a READ memory access. > #0 0x7f525c2c4881 in memcpy string/../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:222 > #1 0x55aa782bd5b1 in __asan_memcpy llvm/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp:22:3 > #2 0x55aa7854dedd in tcx_blit_writel hw/display/tcx.c:590:13 > > Reproducer: > cat << EOF | qemu-system-sparc -display none \ > -machine accel=qtest, -m 512M -machine LX -m 256 -qtest stdio > writel 0x562e98c4 0x3d92fd01 > EOF > > Signed-off-by: Zheyu Ma <zheyuma97@gmail.com> > --- > hw/display/tcx.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/hw/display/tcx.c b/hw/display/tcx.c > index 99507e7638..af43bea7f2 100644 > --- a/hw/display/tcx.c > +++ b/hw/display/tcx.c > @@ -33,6 +33,7 @@ > #include "migration/vmstate.h" > #include "qemu/error-report.h" > #include "qemu/module.h" > +#include "qemu/log.h" > #include "qom/object.h" > > #define TCX_ROM_FILE "QEMU,tcx.bin" > @@ -577,6 +578,14 @@ static void tcx_blit_writel(void *opaque, hwaddr addr, > addr = (addr >> 3) & 0xfffff; > adsr = val & 0xffffff; > len = ((val >> 24) & 0x1f) + 1; > + > + if (addr + len > s->vram_size || adsr + len > s->vram_size) { > + qemu_log_mask(LOG_GUEST_ERROR, > + "%s: VRAM access out of bounds. addr: 0x%lx, adsr: 0x%x, len: %u\n", > + __func__, addr, adsr, len); > + return; > + } > + > if (adsr == 0xffffff) { > memset(&s->vram[addr], s->tmpblit, len); > if (s->depth == 24) { What would happen if the source data plus length goes beyond the end of the framebuffer but the destination lies completely within it? Presumably the length of the data copy should be restricted to the length of the source data rather than the entire copy being ignored? ATB, Mark.
diff --git a/hw/display/tcx.c b/hw/display/tcx.c index 99507e7638..af43bea7f2 100644 --- a/hw/display/tcx.c +++ b/hw/display/tcx.c @@ -33,6 +33,7 @@ #include "migration/vmstate.h" #include "qemu/error-report.h" #include "qemu/module.h" +#include "qemu/log.h" #include "qom/object.h" #define TCX_ROM_FILE "QEMU,tcx.bin" @@ -577,6 +578,14 @@ static void tcx_blit_writel(void *opaque, hwaddr addr, addr = (addr >> 3) & 0xfffff; adsr = val & 0xffffff; len = ((val >> 24) & 0x1f) + 1; + + if (addr + len > s->vram_size || adsr + len > s->vram_size) { + qemu_log_mask(LOG_GUEST_ERROR, + "%s: VRAM access out of bounds. addr: 0x%lx, adsr: 0x%x, len: %u\n", + __func__, addr, adsr, len); + return; + } + if (adsr == 0xffffff) { memset(&s->vram[addr], s->tmpblit, len); if (s->depth == 24) {
This patch addresses a potential out-of-bounds memory access issue in the tcx_blit_writel function. It adds bounds checking to ensure that memory accesses do not exceed the allocated VRAM size. If an out-of-bounds access is detected, an error is logged using qemu_log_mask. ASAN log: ==2960379==ERROR: AddressSanitizer: SEGV on unknown address 0x7f524752fd01 (pc 0x7f525c2c4881 bp 0x7ffdaf87bfd0 sp 0x7ffdaf87b788 T0) ==2960379==The signal is caused by a READ memory access. #0 0x7f525c2c4881 in memcpy string/../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:222 #1 0x55aa782bd5b1 in __asan_memcpy llvm/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp:22:3 #2 0x55aa7854dedd in tcx_blit_writel hw/display/tcx.c:590:13 Reproducer: cat << EOF | qemu-system-sparc -display none \ -machine accel=qtest, -m 512M -machine LX -m 256 -qtest stdio writel 0x562e98c4 0x3d92fd01 EOF Signed-off-by: Zheyu Ma <zheyuma97@gmail.com> --- hw/display/tcx.c | 9 +++++++++ 1 file changed, 9 insertions(+)