Re: [PATCH 1/1] uas: add stream number sanity checks.

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 8/18/21 2:05 PM, 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

Reported-by: Chen Zhe <chenzhe@huawei.com>
Reported-by: Tan Jingguo <tanjingguo@huawei.com>

> Signed-off-by: Gerd Hoffmann <kraxel@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;
>  }

Thanks for tackling this, I was looking but thought it was too
late for 6.1. Beside your obvious checks, I'd also modify
usb_uas_alloc_status() to feel safer, by checking 'tag'.
Eventually returning NULL and adapt the call sites? Anyway
this can wait 6.2, so for this patch:

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>