Re: [PATCH 1/1] USBHID: correct start/stop cycle

[Jiri Kosina <jkosina@suse.cz>]

On Sat, 1 Nov 2008, Jiri Slaby wrote:

> `stop' left out usbhid->urb* pointers and so the next `start' thought
> it needs to allocate nothing and used the memory pointers previously
> pointed to. This led to memory corruption and device malfunction.
> 
> Also don't forget to clear disconnect flag on start which was left set
> by the previous `stop'.
> 
> Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
> ---
>  drivers/hid/usbhid/hid-core.c |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> index 18e5ddd..f0339ae 100644
> --- a/drivers/hid/usbhid/hid-core.c
> +++ b/drivers/hid/usbhid/hid-core.c
> @@ -781,6 +781,8 @@ static int usbhid_start(struct hid_device *hid)
>  	unsigned int n, insize = 0;
>  	int ret;
>  
> +	clear_bit(HID_DISCONNECTED, &usbhid->iofl);
> +
>  	usbhid->bufsize = HID_MIN_BUFFER_SIZE;
>  	hid_find_max_report(hid, HID_INPUT_REPORT, &usbhid->bufsize);
>  	hid_find_max_report(hid, HID_OUTPUT_REPORT, &usbhid->bufsize);
> @@ -888,6 +890,9 @@ fail:
>  	usb_free_urb(usbhid->urbin);
>  	usb_free_urb(usbhid->urbout);
>  	usb_free_urb(usbhid->urbctrl);
> +	usbhid->urbin = NULL;
> +	usbhid->urbout = NULL;
> +	usbhid->urbctrl = NULL;
>  	hid_free_buffers(dev, hid);
>  	mutex_unlock(&usbhid->setup);
>  	return ret;
> @@ -924,6 +929,9 @@ static void usbhid_stop(struct hid_device *hid)
>  	usb_free_urb(usbhid->urbin);
>  	usb_free_urb(usbhid->urbctrl);
>  	usb_free_urb(usbhid->urbout);
> +	usbhid->urbin = NULL; /* don't mess up next start */
> +	usbhid->urbctrl = NULL;
> +	usbhid->urbout = NULL;
>  
>  	hid_free_buffers(hid_to_usb_dev(hid), hid);
>  	mutex_unlock(&usbhid->setup);

Jeroen, Helge,

could you please verify whether this patch fixes the corruption you were 
experiencing?

[ I will be offline for the upcoming ~9 days, will push the fix upstream 
  then, if it is not picked up through different channels in the 
  meantime ]

Thanks!

-- 
Jiri Kosina
SUSE Labs