Re: [PATCH] hw/usb/hcd-dwc2: fix divide-by-zero in dwc2_handle_packet()

[Paul Zimmerman]

On Mon, Oct 12, 2020 at 8:05 AM Mauro Matteo Cascella
<mcascell@redhat.com> wrote:
>
> Check the value of mps before it is used as divisor. Since HCCHAR_MPS is guest
> controllable, this prevents a malicious/buggy guest from crashing the QEMU
> process on the host.
>
> Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com>
> Reported-by: Gaoning Pan <gaoning.pgn@antgroup.com>
> Reported-by: Xingwei Lin <linyi.lxw@antfin.com>
> ---
>  hw/usb/hcd-dwc2.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/hw/usb/hcd-dwc2.c b/hw/usb/hcd-dwc2.c
> index 97688d21bf..91476fd781 100644
> --- a/hw/usb/hcd-dwc2.c
> +++ b/hw/usb/hcd-dwc2.c
> @@ -324,6 +324,12 @@ babble:
>              }
>          }
>
> +        if (mps == 0) {
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                    "%s: Bad HCCHAR_MPS set to zero\n", __func__);
> +            return;
> +        }
> +
>          tpcnt = actual / mps;
>          if (actual % mps) {
>              tpcnt++;
> --
> 2.26.2
>

Hi Mauro,

I think it would be better to move this check earlier in the function,
just after 'mps' is read from the register. Otherwise it can get
assigned to 'tlen' and 'p->mps', and who knows what mischief an
invalid value there might cause.

After that, you can add my
Reviewed-by: Paul Zimmerman <pauldzim@gmail.com>

Thanks,
Paul