On Wed, Aug 18, 2021 at 02:05:05PM +0200, Gerd Hoffmann wrote: > The device uses the guest-supplied stream number unchecked, which can > lead to guest-triggered out-of-band access to the UASDevice->data3 and > UASDevice->status3 fields. Add the missing checks. > > Fixes: CVE-2021-3713 > Signed-off-by: Gerd Hoffmann <kra...@redhat.com> > Reported-by: Chen Zhe <chen...@huawei.com> > Reported-by: Tan Jingguo <tanjing...@huawei.com> > Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com> > --- > hw/usb/dev-uas.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/hw/usb/dev-uas.c b/hw/usb/dev-uas.c > index 263056231c79..f6309a5ebfdc 100644 > --- a/hw/usb/dev-uas.c > +++ b/hw/usb/dev-uas.c > @@ -840,6 +840,9 @@ static void usb_uas_handle_data(USBDevice *dev, USBPacket > *p) > } > break; > case UAS_PIPE_ID_STATUS: > + if (p->stream > UAS_MAX_STREAMS) { > + goto err_stream; > + } > if (p->stream) { > QTAILQ_FOREACH(st, &uas->results, next) { > if (st->stream == p->stream) { > @@ -867,6 +870,9 @@ static void usb_uas_handle_data(USBDevice *dev, USBPacket > *p) > break; > case UAS_PIPE_ID_DATA_IN: > case UAS_PIPE_ID_DATA_OUT: > + if (p->stream > UAS_MAX_STREAMS) { > + goto err_stream; > + } > if (p->stream) { > req = usb_uas_find_request(uas, p->stream); > } else { > @@ -902,6 +908,11 @@ static void usb_uas_handle_data(USBDevice *dev, > USBPacket *p) > p->status = USB_RET_STALL; > break; > } > + > +err_stream: > + error_report("%s: invalid stream %d", __func__, p->stream); > + p->status = USB_RET_STALL; > + return;
How is this supposed to work ? It results in messages such as the following. qemu-system-sparc64: usb_uas_handle_data: invalid stream 1 qemu-system-sparc64: usb_uas_handle_data: invalid stream 1 It also sets the status unconditionally to USB_RET_STALL, and UAS is simply broken after this patch is applied because the error handling code is executed literally for each call of usb_uas_handle_data(). Guenter