Re: [Qemu-devel] [PATCH 1/5] usb-linux.c: allow full-size control transfers, do not overrun buffer

[malc <av1474@comtv.ru>]

On Tue, 3 Feb 2009, Ian Jackson wrote:

> The buffer in struct ctrl_struct needs to be big enough for any
> control transfer which may be initiated by the guest, since we are
> perhaps trying to pass a device through.  The biggest possible size is
> 2^16-1 since the length fields are 16 bits.
> 
> Also, assert that the transfer request we are about to make to our
> host kernel will not overrun the buffer.
> 
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
> ---
>  usb-linux.c |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/usb-linux.c b/usb-linux.c
> index fb1153b..321c1db 100644
> --- a/usb-linux.c
> +++ b/usb-linux.c
> @@ -34,6 +34,8 @@
>  #include "qemu-timer.h"
>  #include "console.h"
>  
> +#if defined(__linux__)

And endif is... (Leaving aside the fact that __linux__ guard in a file
called something-linux.c is a weird looking thing)

> +#include <assert.h>
>  #include <dirent.h>
>  #include <sys/ioctl.h>
>  #include <signal.h>
> @@ -115,7 +117,7 @@ struct ctrl_struct {
>      uint16_t offset;
>      uint8_t  state;
>      struct   usb_ctrlrequest req;
> -    uint8_t  buffer[1024];
> +    uint8_t  buffer[65536];
>  };
>  
>  typedef struct USBHostDevice {
> @@ -603,6 +605,8 @@ static int usb_host_handle_control(USBHostDevice *s, USBPacket *p)
>      urb->type     = USBDEVFS_URB_TYPE_CONTROL;
>      urb->endpoint = p->devep;
>  
> +    assert(s->ctrl.len < sizeof(s->ctrl.buffer));
> +

If something can happen it will, if assert can turn into a nop it will do
so also, `if (cond) abort();' is more apropriate.

>      urb->buffer        = &s->ctrl.req;
>      urb->buffer_length = 8 + s->ctrl.len;
>  
> 

-- 
mailto:av1474@comtv.ru