Re: [Qemu-devel] [PATCH v4 24/30] usb: sanity check setup_index+setup_len in post_load

[Peter Maydell]

On 31 March 2014 15:17, Michael S. Tsirkin <address@hidden> wrote:
> From: Gerd Hoffmann <address@hidden>
>
> CVE-2013-4541
>
> s->setup_len and s->setup_index are fed into usb_packet_copy as
> size/offset into s->data_buf, it's possible for invalid state to exploit
> this to load arbitrary data.
>
> setup_len and setup_index should be checked against data_buf size.
>
> Signed-off-by: Gerd Hoffmann <address@hidden>
> Signed-off-by: Michael S. Tsirkin <address@hidden>
> ---
>  hw/usb/bus.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/hw/usb/bus.c b/hw/usb/bus.c
> index fe70429..8052bf1 100644
> --- a/hw/usb/bus.c
> +++ b/hw/usb/bus.c
> @@ -53,6 +53,10 @@ static int usb_device_post_load(void *opaque, int 
> version_id)
>          dev->setup_len >= sizeof(dev->data_buf)) {
>          return -EINVAL;
>      }
> +    if (dev->setup_index >= sizeof(dev->data_buf) ||
> +        dev->setup_len >= sizeof(dev->data_buf)) {
> +        return -EINVAL;
> +    }
>      return 0;
>  }

(1) This patch has already been applied; looks like a rebase
merge error meant you failed to drop it.
(2) Shouldn't we be checking for setup_index and setup_len
being negative as well?

thanks
-- PMM