All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RESEND] USB HID: Protect against disconnect/NULL-dereference race
       [not found] <m3y6ee62gp.fsf@pullcord.laptop.org>
@ 2010-08-12 23:07 ` Chris Ball
       [not found]   ` <m3lj8b8m6b.fsf-0VGQAjvlmrQzNDMTQreKSUB+6BGkLq7r@public.gmane.org>
  0 siblings, 1 reply; 3+ messages in thread
From: Chris Ball @ 2010-08-12 23:07 UTC (permalink / raw)
  To: linux-usb; +Cc: Jiri Kosina, linux-input

One of our users reports consistently hitting a NULL dereference that
resolves to the "hid_to_usb_dev(hid);" call in hiddev_ioctl(), when
disconnecting a Lego WeDo USB HID device from an OLPC XO running
Scratch software.  There's a FIXME comment and a guard against the
dereference, but that happens farther down the function than the
initial dereference does.

This patch moves the call to be below the guard, and the user reports
that it fixes the problem for him.  OLPC bug report:
http://dev.laptop.org/ticket/10174

Signed-off-by: Chris Ball <cjb@laptop.org>
---
 drivers/hid/usbhid/hiddev.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
index c24d2fa..7ec009a 100644
--- a/drivers/hid/usbhid/hiddev.c
+++ b/drivers/hid/usbhid/hiddev.c
@@ -593,7 +593,7 @@ static long hiddev_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	struct hiddev_list *list = file->private_data;
 	struct hiddev *hiddev = list->hiddev;
 	struct hid_device *hid = hiddev->hid;
-	struct usb_device *dev = hid_to_usb_dev(hid);
+	struct usb_device *dev;
 	struct hiddev_collection_info cinfo;
 	struct hiddev_report_info rinfo;
 	struct hiddev_field_info finfo;
@@ -607,9 +607,11 @@ static long hiddev_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	/* Called without BKL by compat methods so no BKL taken */
 
 	/* FIXME: Who or what stop this racing with a disconnect ?? */
-	if (!hiddev->exist)
+	if (!hiddev->exist || !hid)
 		return -EIO;
 
+	dev = hid_to_usb_dev(hid);
+
 	switch (cmd) {
 
 	case HIDIOCGVERSION:
-- 
1.7.0.1

-- 
Chris Ball   <cjb@laptop.org>
One Laptop Per Child

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH RESEND] USB HID: Protect against disconnect/NULL-dereference race
       [not found]   ` <m3lj8b8m6b.fsf-0VGQAjvlmrQzNDMTQreKSUB+6BGkLq7r@public.gmane.org>
@ 2010-08-13  8:55     ` Amit Nagal
  2010-08-13  9:21     ` Jiri Kosina
  1 sibling, 0 replies; 3+ messages in thread
From: Amit Nagal @ 2010-08-13  8:55 UTC (permalink / raw)
  To: linux-usb-u79uwXL29TY76Z2rM5mHXA
  Cc: Jiri Kosina, linux-input-u79uwXL29TY76Z2rM5mHXA, Chris Ball

Hi ,

i am also using HIDDEV interface . and i also got hiddev_ioctl NULL
dereference problem .

Below  patch fixes my problem .

Regards
Amit Nagal







On Fri, Aug 13, 2010 at 4:37 AM, Chris Ball <cjb-2X9k7bc8m7Mdnm+yROfE0A@public.gmane.org> wrote:
> One of our users reports consistently hitting a NULL dereference that
> resolves to the "hid_to_usb_dev(hid);" call in hiddev_ioctl(), when
> disconnecting a Lego WeDo USB HID device from an OLPC XO running
> Scratch software.  There's a FIXME comment and a guard against the
> dereference, but that happens farther down the function than the
> initial dereference does.
>
> This patch moves the call to be below the guard, and the user reports
> that it fixes the problem for him.  OLPC bug report:
> http://dev.laptop.org/ticket/10174
>
> Signed-off-by: Chris Ball <cjb-2X9k7bc8m7Mdnm+yROfE0A@public.gmane.org>
> ---
>  drivers/hid/usbhid/hiddev.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
> index c24d2fa..7ec009a 100644
> --- a/drivers/hid/usbhid/hiddev.c
> +++ b/drivers/hid/usbhid/hiddev.c
> @@ -593,7 +593,7 @@ static long hiddev_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>        struct hiddev_list *list = file->private_data;
>        struct hiddev *hiddev = list->hiddev;
>        struct hid_device *hid = hiddev->hid;
> -       struct usb_device *dev = hid_to_usb_dev(hid);
> +       struct usb_device *dev;
>        struct hiddev_collection_info cinfo;
>        struct hiddev_report_info rinfo;
>        struct hiddev_field_info finfo;
> @@ -607,9 +607,11 @@ static long hiddev_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>        /* Called without BKL by compat methods so no BKL taken */
>
>        /* FIXME: Who or what stop this racing with a disconnect ?? */
> -       if (!hiddev->exist)
> +       if (!hiddev->exist || !hid)
>                return -EIO;
>
> +       dev = hid_to_usb_dev(hid);
> +
>        switch (cmd) {
>
>        case HIDIOCGVERSION:
> --
> 1.7.0.1
>
> --
> Chris Ball   <cjb-2X9k7bc8m7Mdnm+yROfE0A@public.gmane.org>
> One Laptop Per Child
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH RESEND] USB HID: Protect against disconnect/NULL-dereference race
       [not found]   ` <m3lj8b8m6b.fsf-0VGQAjvlmrQzNDMTQreKSUB+6BGkLq7r@public.gmane.org>
  2010-08-13  8:55     ` Amit Nagal
@ 2010-08-13  9:21     ` Jiri Kosina
  1 sibling, 0 replies; 3+ messages in thread
From: Jiri Kosina @ 2010-08-13  9:21 UTC (permalink / raw)
  To: Chris Ball
  Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-input-u79uwXL29TY76Z2rM5mHXA

On Thu, 12 Aug 2010, Chris Ball wrote:

> One of our users reports consistently hitting a NULL dereference that
> resolves to the "hid_to_usb_dev(hid);" call in hiddev_ioctl(), when
> disconnecting a Lego WeDo USB HID device from an OLPC XO running
> Scratch software.  There's a FIXME comment and a guard against the
> dereference, but that happens farther down the function than the
> initial dereference does.
> 
> This patch moves the call to be below the guard, and the user reports
> that it fixes the problem for him.  OLPC bug report:
> http://dev.laptop.org/ticket/10174
> 
> Signed-off-by: Chris Ball <cjb-2X9k7bc8m7Mdnm+yROfE0A@public.gmane.org>

Applied, thanks Chris.

-- 
Jiri Kosina
SUSE Labs, Novell Inc.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2010-08-13  9:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <m3y6ee62gp.fsf@pullcord.laptop.org>
2010-08-12 23:07 ` [PATCH RESEND] USB HID: Protect against disconnect/NULL-dereference race Chris Ball
     [not found]   ` <m3lj8b8m6b.fsf-0VGQAjvlmrQzNDMTQreKSUB+6BGkLq7r@public.gmane.org>
2010-08-13  8:55     ` Amit Nagal
2010-08-13  9:21     ` Jiri Kosina

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.