[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
答复: [PATCH] usb/dev-wacom: fix OOB write in usb_mouse_poll()
From: |
ningqiang (A) |
Subject: |
答复: [PATCH] usb/dev-wacom: fix OOB write in usb_mouse_poll() |
Date: |
Thu, 16 Feb 2023 01:05:52 +0000 |
Qemu cmd and guest poc
/home/test/qemu/qemu-7.1.0/build/qemu-system-x86_64 -kernel
/home/test/kernel/linux-5.10/arch/x86/boot/bzImage -initrd
/home/test/rootfs/rootfs.cpio_root -append "root=/dev/ram rw console=tty
console=ttyS0 nokaslr" -m 512M -nographic -monitor /dev/null -drive
file=null-co://,if=none,format=raw,id=disk0 -device qemu-xhci,id=xhci -device
usb-tablet,bus=xhci.0 -device usb-mouse -device usb-wacom-tablet
-----邮件原件-----
发件人: Mauro Matteo Cascella [mailto:mcascell@redhat.com]
发送时间: 2023年2月14日 18:48
收件人: Philippe Mathieu-Daudé <philmd@linaro.org>
抄送: qemu-devel@nongnu.org; kraxel@redhat.com; ningqiang (A)
<ningqiang1@huawei.com>; soul chen <soulchen8650@gmail.com>
主题: Re: [PATCH] usb/dev-wacom: fix OOB write in usb_mouse_poll()
Hi Philippe,
On Mon, Feb 13, 2023 at 7:26 PM Philippe Mathieu-Daudé <philmd@linaro.org>
wrote:
>
> Hi Mauro,
>
> On 13/2/23 18:41, Mauro Matteo Cascella wrote:
> > The guest can control the size of buf; an OOB write occurs when buf
> > is 1 or 2 bytes long. Only fill in the buffer as long as there is
> > enough space, throw away any data which doesn't fit.
>
> Any reproducer?
No qtest reproducer, we do have a PoC module to compile & load from within the
guest. This is ASAN output:
=================================================================
==28803==ERROR: AddressSanitizer: heap-buffer-overflow on address 0 WRITE of
size 1 at 0x602000fccdd1 thread T2
#0 0x560f4ebba899 in usb_mouse_poll ../hw/usb/dev-wacom.c:256
#1 0x560f4ebbcaf9 in usb_wacom_handle_data ../hw/usb/dev-wacom6
#2 0x560f4eaef297 in usb_device_handle_data ../hw/usb/bus.c:180
#3 0x560f4eb00bbb in usb_process_one ../hw/usb/core.c:406
#4 0x560f4eb01883 in usb_handle_packet ../hw/usb/core.c:438
#5 0x560f4eb94e0c in xhci_submit ../hw/usb/hcd-xhci.c:1801
#6 0x560f4eb9505f in xhci_fire_transfer ../hw/usb/hcd-xhci.c:10
#7 0x560f4eb9773c in xhci_kick_epctx ../hw/usb/hcd-xhci.c:1969
#8 0x560f4eb953f2 in xhci_kick_ep ../hw/usb/hcd-xhci.c:1835
#9 0x560f4eba416d in xhci_doorbell_write ../hw/usb/hcd-xhci.c:7
#10 0x560f4f5343a8 in memory_region_write_accessor ../softmmu/2
#11 0x560f4f53483f in access_with_adjusted_size ../softmmu/mem4
#12 0x560f4f541e69 in memory_region_dispatch_write ../softmmu/4
#13 0x560f4f57afec in flatview_write_continue ../softmmu/physm5
#14 0x560f4f57b40f in flatview_write ../softmmu/physmem.c:2867
#15 0x560f4f579617 in subpage_write ../softmmu/physmem.c:2501
#16 0x560f4f5346dc in memory_region_write_with_attrs_accessor 3
#17 0x560f4f53483f in access_with_adjusted_size ../softmmu/mem4
#18 0x560f4f542075 in memory_region_dispatch_write ../softmmu/1
#19 0x560f4f727735 in io_writex ../accel/tcg/cputlb.c:1429
#20 0x560f4f72c19d in store_helper ../accel/tcg/cputlb.c:2379
#21 0x560f4f72c5ec in full_le_stl_mmu ../accel/tcg/cputlb.c:247
#22 0x560f4f72c62a in helper_le_stl_mmu ../accel/tcg/cputlb.c:3
#23 0x7fcf063941a3 (/memfd:tcg-jit (deleted)+0x27541a3)
<cut>
Also forgot to give credits:
Reported-by: ningqiang1 <ningqiang1@huawei.com>
Reported-by: SorryMybad of Kunlun Lab <soulchen8650@gmail.com>
> > Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com>
> > ---
> > hw/usb/dev-wacom.c | 20 +++++++++++++-------
> > 1 file changed, 13 insertions(+), 7 deletions(-)
> >
> > diff --git a/hw/usb/dev-wacom.c b/hw/usb/dev-wacom.c index
> > 7177c17f03..ca9e6aa82f 100644
> > --- a/hw/usb/dev-wacom.c
> > +++ b/hw/usb/dev-wacom.c
> > @@ -252,14 +252,20 @@ static int usb_mouse_poll(USBWacomState *s, uint8_t
> > *buf, int len)
> > if (s->buttons_state & MOUSE_EVENT_MBUTTON)
> > b |= 0x04;
> >
> > - buf[0] = b;
> > - buf[1] = dx;
> > - buf[2] = dy;
> > - l = 3;
> > - if (len >= 4) {
> > - buf[3] = dz;
> > - l = 4;
> > + l = 0;
> > + if (len > l) {
> > + buf[l++] = b;
> > }
> > + if (len > l) {
> > + buf[l++] = dx;
> > + }
>
> else { // the packet is now corrupted... }
>
> > + if (len > l) {
> > + buf[l++] = dy;
> > + }
> > + if (len > l) {
> > + buf[l++] = dz;
> > + }
> > +
> > return l;
> > }
>
> Better is to wait for enough data to process:
>
> -- >8 --
> diff --git a/hw/usb/dev-wacom.c b/hw/usb/dev-wacom.c index
> 7177c17f03..2fe2a9220e 100644
> --- a/hw/usb/dev-wacom.c
> +++ b/hw/usb/dev-wacom.c
> @@ -244,6 +244,9 @@ static int usb_mouse_poll(USBWacomState *s,
> uint8_t *buf, int len)
> s->dy -= dy;
> s->dz -= dz;
>
> + if (len < 3)
> + return 0;
> +
> b = 0;
> if (s->buttons_state & MOUSE_EVENT_LBUTTON)
> b |= 0x01;
> @@ -274,6 +277,9 @@ static int usb_wacom_poll(USBWacomState *s,
> uint8_t *buf, int len)
> s->mouse_grabbed = 1;
> }
>
> + if (len < 7)
> + return 0;
> +
> b = 0;
> if (s->buttons_state & MOUSE_EVENT_LBUTTON)
> b |= 0x01;
> @@ -282,9 +288,6 @@ static int usb_wacom_poll(USBWacomState *s,
> uint8_t *buf, int len)
> if (s->buttons_state & MOUSE_EVENT_MBUTTON)
> b |= 0x20; /* eraser */
>
> - if (len < 7)
> - return 0;
> -
> buf[0] = s->mode;
> buf[5] = 0x00 | (b & 0xf0);
> buf[1] = s->x & 0xff;
> ---
>
I took inspiration from hid_pointer_poll() in hw/input/hid.c which fills in the
buffer as much as possible in a similar way, but your suggestion makes sense to
me. Gerd, wdyt?
Thanks,
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
usb-mouse-heap-oob-write-poc.c
Description: usb-mouse-heap-oob-write-poc.c