Hello,
The merge window is closed so it's time to hunt for bugs. I've got
one, please find details below.
-----------------------
Working kernels: from 2.6.35 - to 2.6.35.2
Non working kernels: from (at least) 2.6.35-git2 - to 2.6.36-rc1
-----------------------
Description of the problem(s):
1. First of all I confirm that the problem is consistent: suspend
works well every time on the 'good' kernels and it newer works (with
the symptoms) on any of the 'bad' kernels. I've tested it several
times on a couple of 'good' and 'bad' kernels to make sure it's not
some random issue.
2. On a bad kernel, this is what happens when I try to switch to suspend mode:
- screen goes blank ['No signal detected !'] - that's OK :)
- either the CPU fan or PSU fan or both are still running and they
won't stop (left it for about 30 min. once)
- the power light is on and still, where in suspend mode it should
blink - that won't change either
- pressing the power button obviously doesn't wake the system up; need
to hard reboot
3. Now, when booting again, after an unsuccessful suspend and hard
reboot, something happens to my network card (and it doesn't matter
what is the next kernel I boot from - it can be any 'good' or 'bad'
one):
- in Gnome, the gnome-panel network indicator shows my connection as
disabled (wired eth0 - the only one I use); enabling it doesn't bring
my network up
4. After re-booting from any of the 'good' or 'bad' kernels my network
connection is working again.
-----------------------
Bisecting turned up the following commit. Reverting it in 2.6.36-rc1
results in a system that works.
commit bd25f4dd6972755579d0ea50d1a5ace2e9b00d1a
Author: Arnd Bergmann <[email protected]>
Date:?? Sun Jul 11 15:34:05 2010 +0200
??? HID: hiddev: use usb_find_interface, get rid of BKL
??? This removes the private hiddev_table in the usbhid
??? driver and changes it to use usb_find_interface
??? instead.
??? The advantage is that we can avoid the race between
??? usb_register_dev and usb_open and no longer need the
??? big kernel lock.
??? This doesn't introduce race condition -- the intf pointer could be
??? invalidated only in hiddev_disconnect() through usb_deregister_dev(),
??? but that will block on minor_rwsem and not actually remove the device
??? until usb_open().
-----------------------
Despite mentioned issues with network (occurring every time after
unsuccessful suspend > hard restart) the bisecting turned up a commit
which isn't rather related to network. But I can confirm that
reverting only this one commit in 2.6.36-rc1
[da5cabf80e2433131bf0ed8993abc0f7ea618c73] fixes the whole problem -
suspend works OK and I have no issues with my network after waking the
system up, neither after re-booting from the same or any other kernel.
Please let me know what further details should I provide. Any logs,
hardware specifications? I've also made a brief log recording the
bisect if anyone would need it.
--
Regards
Tomasz B. aka trapDoor
Posted on Bugzilla as well, ref: 16615 - [REGRESSION, bisected]
2.6.36-rc1 - suspend doesn't work and network issues afterwards
--
Thanks
Tomasz B.
On Tue, Aug 17, 2010 at 6:01 PM, trapDoor <[email protected]> wrote:
> Hello,
> The merge window is closed so it's time to hunt for bugs. I've got
> one, please find details below.
> -----------------------
>
> Working kernels: from 2.6.35 - to 2.6.35.2
> Non working kernels: from (at least) 2.6.35-git2 - to 2.6.36-rc1
> -----------------------
>
> Description of the problem(s):
>
> 1. First of all I confirm that the problem is consistent: suspend
> works well every time on the 'good' kernels and it newer works (with
> the symptoms) on any of the 'bad' kernels. I've tested it several
> times on a couple of 'good' and 'bad' kernels to make sure it's not
> some random issue.
>
> 2. On a bad kernel, this is what happens when I try to switch to suspend mode:
> - screen goes blank ['No signal detected !'] - that's OK :)
> - either the CPU fan or PSU fan or both are still running and they
> won't stop (left it for about 30 min. once)
> - the power light is on and still, where in suspend mode it should
> blink - that won't change either
> - pressing the power button obviously doesn't wake the system up; need
> to hard reboot
>
> 3. Now, when booting again, after an unsuccessful suspend and hard
> reboot, something happens to my network card (and it doesn't matter
> what is the next kernel I boot from - it can be any 'good' or 'bad'
> one):
> - in Gnome, the gnome-panel network indicator shows my connection as
> disabled (wired eth0 - the only one I use); enabling it doesn't bring
> my network up
>
> 4. After re-booting from any of the 'good' or 'bad' kernels my network
> connection is working again.
> -----------------------
>
> Bisecting turned up the following commit. Reverting it in 2.6.36-rc1
> results in a system that works.
>
> commit bd25f4dd6972755579d0ea50d1a5ace2e9b00d1a
> Author: Arnd Bergmann <[email protected]>
> Date:?? Sun Jul 11 15:34:05 2010 +0200
>
> ??? HID: hiddev: use usb_find_interface, get rid of BKL
>
> ??? This removes the private hiddev_table in the usbhid
> ??? driver and changes it to use usb_find_interface
> ??? instead.
>
> ??? The advantage is that we can avoid the race between
> ??? usb_register_dev and usb_open and no longer need the
> ??? big kernel lock.
>
> ??? This doesn't introduce race condition -- the intf pointer could be
> ??? invalidated only in hiddev_disconnect() through usb_deregister_dev(),
> ??? but that will block on minor_rwsem and not actually remove the device
> ??? until usb_open().
> -----------------------
>
> Despite mentioned issues with network (occurring every time after
> unsuccessful suspend > hard restart) the bisecting turned up a commit
> which isn't rather related to network. But I can confirm that
> reverting only this one commit in 2.6.36-rc1
> [da5cabf80e2433131bf0ed8993abc0f7ea618c73] fixes the whole problem -
> suspend works OK and I have no issues with my network after waking the
> system up, neither after re-booting from the same or any other kernel.
>
> Please let me know what further details should I provide. Any logs,
> hardware specifications? I've also made a brief log recording the
> bisect if anyone would need it.
>
> --
> Regards
> Tomasz B. aka trapDoor
>
--
Regards
trapDoor
2010/8/17 trapDoor <[email protected]>:
> Hello,
> The merge window is closed so it's time to hunt for bugs. I've got
> one, please find details below.
Same problem here...
But I've made no bisecting.
Thanks a lot,
Andrea
Thanks Andrea.
Have you tried to revert the commit to see if it fixes the issue on your end?
I also have response from Gabriel Craciunescu on bugzilla
[https://bugzilla.kernel.org/show_bug.cgi?id=16615], who had the same
problem and confirmed that reverting the same commit solved it.
--
Regards
Tomasz B.
On Tue, Aug 17, 2010 at 9:54 PM, Andrea Gelmini
<[email protected]> wrote:
> 2010/8/17 trapDoor <[email protected]>:
>> Hello,
>> The merge window is closed so it's time to hunt for bugs. I've got
>> one, please find details below.
>
> Same problem here...
> But I've made no bisecting.
>
> Thanks a lot,
> Andrea
>
On Tuesday, August 17, 2010, trapDoor wrote:
> Hello,
> The merge window is closed so it's time to hunt for bugs. I've got
> one, please find details below.
> -----------------------
>
> Working kernels: from 2.6.35 - to 2.6.35.2
> Non working kernels: from (at least) 2.6.35-git2 - to 2.6.36-rc1
> -----------------------
>
> Description of the problem(s):
>
> 1. First of all I confirm that the problem is consistent: suspend
> works well every time on the 'good' kernels and it newer works (with
> the symptoms) on any of the 'bad' kernels. I've tested it several
> times on a couple of 'good' and 'bad' kernels to make sure it's not
> some random issue.
>
> 2. On a bad kernel, this is what happens when I try to switch to suspend mode:
> - screen goes blank ['No signal detected !'] - that's OK :)
> - either the CPU fan or PSU fan or both are still running and they
> won't stop (left it for about 30 min. once)
> - the power light is on and still, where in suspend mode it should
> blink - that won't change either
> - pressing the power button obviously doesn't wake the system up; need
> to hard reboot
>
> 3. Now, when booting again, after an unsuccessful suspend and hard
> reboot, something happens to my network card (and it doesn't matter
> what is the next kernel I boot from - it can be any 'good' or 'bad'
> one):
> - in Gnome, the gnome-panel network indicator shows my connection as
> disabled (wired eth0 - the only one I use); enabling it doesn't bring
> my network up
>
> 4. After re-booting from any of the 'good' or 'bad' kernels my network
> connection is working again.
> -----------------------
>
> Bisecting turned up the following commit. Reverting it in 2.6.36-rc1
> results in a system that works.
>
> commit bd25f4dd6972755579d0ea50d1a5ace2e9b00d1a
> Author: Arnd Bergmann <[email protected]>
> Date: Sun Jul 11 15:34:05 2010 +0200
>
> HID: hiddev: use usb_find_interface, get rid of BKL
>
> This removes the private hiddev_table in the usbhid
> driver and changes it to use usb_find_interface
> instead.
>
> The advantage is that we can avoid the race between
> usb_register_dev and usb_open and no longer need the
> big kernel lock.
>
> This doesn't introduce race condition -- the intf pointer could be
> invalidated only in hiddev_disconnect() through usb_deregister_dev(),
> but that will block on minor_rwsem and not actually remove the device
> until usb_open().
> -----------------------
>
> Despite mentioned issues with network (occurring every time after
> unsuccessful suspend > hard restart) the bisecting turned up a commit
> which isn't rather related to network. But I can confirm that
> reverting only this one commit in 2.6.36-rc1
> [da5cabf80e2433131bf0ed8993abc0f7ea618c73] fixes the whole problem -
> suspend works OK and I have no issues with my network after waking the
> system up, neither after re-booting from the same or any other kernel.
>
> Please let me know what further details should I provide. Any logs,
> hardware specifications? I've also made a brief log recording the
> bisect if anyone would need it.
Well, we should let the HID maintainer know about the problem (CC added).
Thanks,
Rafael
On Wed, Aug 18, 2010 at 2:25 AM, Rafael J. Wysocki <[email protected]> wrote:
> On Tuesday, August 17, 2010, trapDoor wrote:
>> Hello,
>> The merge window is closed so it's time to hunt for bugs. I've got
>> one, please find details below.
>> -----------------------
>>
>> Working kernels: from 2.6.35 - to 2.6.35.2
>> Non working kernels: from (at least) 2.6.35-git2 - to 2.6.36-rc1
>> -----------------------
>>
>> Description of the problem(s):
>>
>> 1. First of all I confirm that the problem is consistent: suspend
>> works well every time on the 'good' kernels and it newer works (with
>> the symptoms) on any of the 'bad' kernels. I've tested it several
>> times on a couple of 'good' and 'bad' kernels to make sure it's not
>> some random issue.
>>
>> 2. On a bad kernel, this is what happens when I try to switch to suspend mode:
>> - screen goes blank ['No signal detected !'] - that's OK :)
>> - either the CPU fan or PSU fan or both are still running and they
>> won't stop (left it for about 30 min. once)
>> - the power light is on and still, where in suspend mode it should
>> blink - that won't change either
>> - pressing the power button obviously doesn't wake the system up; need
>> to hard reboot
>>
>> 3. Now, when booting again, after an unsuccessful suspend and hard
>> reboot, something happens to my network card (and it doesn't matter
>> what is the next kernel I boot from - it can be any 'good' or 'bad'
>> one):
>> - in Gnome, the gnome-panel network indicator shows my connection as
>> disabled (wired eth0 - the only one I use); enabling it doesn't bring
>> my network up
>>
>> 4. After re-booting from any of the 'good' or 'bad' kernels my network
>> connection is working again.
>> -----------------------
>>
>> Bisecting turned up the following commit. Reverting it in 2.6.36-rc1
>> results in a system that works.
>>
>> commit bd25f4dd6972755579d0ea50d1a5ace2e9b00d1a
>> Author: Arnd Bergmann <[email protected]>
>> Date: ? Sun Jul 11 15:34:05 2010 +0200
>>
>> ? ? HID: hiddev: use usb_find_interface, get rid of BKL
>>
>> ? ? This removes the private hiddev_table in the usbhid
>> ? ? driver and changes it to use usb_find_interface
>> ? ? instead.
>>
>> ? ? The advantage is that we can avoid the race between
>> ? ? usb_register_dev and usb_open and no longer need the
>> ? ? big kernel lock.
>>
>> ? ? This doesn't introduce race condition -- the intf pointer could be
>> ? ? invalidated only in hiddev_disconnect() through usb_deregister_dev(),
>> ? ? but that will block on minor_rwsem and not actually remove the device
>> ? ? until usb_open().
>> -----------------------
>>
>> Despite mentioned issues with network (occurring every time after
>> unsuccessful suspend > hard restart) the bisecting turned up a commit
>> which isn't rather related to network. But I can confirm that
>> reverting only this one commit in 2.6.36-rc1
>> [da5cabf80e2433131bf0ed8993abc0f7ea618c73] fixes the whole problem -
>> suspend works OK and I have no issues with my network after waking the
>> system up, neither after re-booting from the same or any other kernel.
>>
>> Please let me know what further details should I provide. Any logs,
>> hardware specifications? I've also made a brief log recording the
>> bisect if anyone would need it.
>
> Well, we should let the HID maintainer know about the problem (CC added).
>
> Thanks,
> Rafael
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at ?http://www.tux.org/lkml/
>
Thanks Rafael,
Before I opened this thread I only searched on
http://vger.kernel.org/vger-lists.html for any appropriate mailing
list but nothing linke linux-*hid* there. Now I've found out that
there is a list of all maintainers included in the kernel source. I'll
keep that in mind ..
--
Regards
Tomasz
On Wed, 18 Aug 2010, trapDoor wrote:
> >> Bisecting turned up the following commit. Reverting it in 2.6.36-rc1
> >> results in a system that works.
> >>
> >> commit bd25f4dd6972755579d0ea50d1a5ace2e9b00d1a
> >> Author: Arnd Bergmann <[email protected]>
> >> Date: ? Sun Jul 11 15:34:05 2010 +0200
> >>
> >> ? ? HID: hiddev: use usb_find_interface, get rid of BKL
> >>
> >> ? ? This removes the private hiddev_table in the usbhid
> >> ? ? driver and changes it to use usb_find_interface
> >> ? ? instead.
> >>
> >> ? ? The advantage is that we can avoid the race between
> >> ? ? usb_register_dev and usb_open and no longer need the
> >> ? ? big kernel lock.
> >>
> >> ? ? This doesn't introduce race condition -- the intf pointer could be
> >> ? ? invalidated only in hiddev_disconnect() through usb_deregister_dev(),
> >> ? ? but that will block on minor_rwsem and not actually remove the device
> >> ? ? until usb_open().
> >> -----------------------
> >>
> >> Despite mentioned issues with network (occurring every time after
> >> unsuccessful suspend > hard restart) the bisecting turned up a commit
> >> which isn't rather related to network. But I can confirm that
> >> reverting only this one commit in 2.6.36-rc1
> >> [da5cabf80e2433131bf0ed8993abc0f7ea618c73] fixes the whole problem -
> >> suspend works OK and I have no issues with my network after waking the
> >> system up, neither after re-booting from the same or any other kernel.
> >>
> >> Please let me know what further details should I provide. Any logs,
> >> hardware specifications? I've also made a brief log recording the
> >> bisect if anyone would need it.
> >
> > Well, we should let the HID maintainer know about the problem (CC added).
>
> Thanks Rafael,
> Before I opened this thread I only searched on
> http://vger.kernel.org/vger-lists.html for any appropriate mailing
> list but nothing linke linux-*hid* there. Now I've found out that
> there is a list of all maintainers included in the kernel source. I'll
> keep that in mind ..
Yeah, MAINTAINERS file is always good to consult. Also CCing all people
who have Signed-off-by or Acked-by the offending patch is usually a good
idea.
Could you please verify whether the patch below (which is currently queued
in my tree and I am planning to push it to Linus soon, as it fixes memory
corruption) fixes your issue? Thanks.
commit 9c9e54a8df0be48aa359744f412377cc55c3b7d2
Author: Jiri Kosina <[email protected]>
Date: Fri Aug 13 12:19:45 2010 +0200
HID: hiddev: fix memory corruption due to invalid intfdata
Commit bd25f4dd6972755579d0 ("HID: hiddev: use usb_find_interface,
get rid of BKL") introduced using of private intfdata in hiddev for
purpose of storing hiddev pointer.
This is a problem, because intf pointer is already being set to struct
hid_device pointer by HID core. This obviously lead to memory corruptions
at device disconnect time, such as
WARNING: at lib/kobject.c:595 kobject_put+0x37/0x4b()
kobject: '(null)' (ffff88011e9cd898): is not initialized, yet kobject_put() is being called.
Convert hiddev into accessing hiddev through struct hid_device which is
in intfdata already.
Reported-and-tested-by: Markus Trippelsdorf <[email protected]>
Reported-and-tested-by: Heinz Diehl <[email protected]>
Reported-and-tested-by: Alan Ott <[email protected]>
Signed-off-by: Jiri Kosina <[email protected]>
diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
index f285017..0a29c51 100644
--- a/drivers/hid/usbhid/hiddev.c
+++ b/drivers/hid/usbhid/hiddev.c
@@ -266,13 +266,15 @@ static int hiddev_open(struct inode *inode, struct file *file)
{
struct hiddev_list *list;
struct usb_interface *intf;
+ struct hid_device *hid;
struct hiddev *hiddev;
int res;
intf = usb_find_interface(&hiddev_driver, iminor(inode));
if (!intf)
return -ENODEV;
- hiddev = usb_get_intfdata(intf);
+ hid = usb_get_intfdata(intf);
+ hiddev = hid->hiddev;
if (!(list = kzalloc(sizeof(struct hiddev_list), GFP_KERNEL)))
return -ENOMEM;
@@ -890,7 +892,6 @@ int hiddev_connect(struct hid_device *hid, unsigned int force)
hid->hiddev = hiddev;
hiddev->hid = hid;
hiddev->exist = 1;
- usb_set_intfdata(usbhid->intf, usbhid);
retval = usb_register_dev(usbhid->intf, &hiddev_class);
if (retval) {
err_hid("Not able to get a minor for this device.");
--
Jiri Kosina
SUSE Labs, Novell Inc.
[adding Gabriel Craciunescu <[email protected]> and Andrea Gelmini
<[email protected]> to CC lilst]
Hello,
On Wed, Aug 18, 2010 at 8:13 AM, Jiri Kosina <[email protected]> wrote:
> On Wed, 18 Aug 2010, trapDoor wrote:
>
>> >> Bisecting turned up the following commit. Reverting it in 2.6.36-rc1
>> >> results in a system that works.
>> >>
>> >> commit bd25f4dd6972755579d0ea50d1a5ace2e9b00d1a
>> >> Author: Arnd Bergmann <[email protected]>
>> >> Date: ? Sun Jul 11 15:34:05 2010 +0200
>> >>
>> >> ? ? HID: hiddev: use usb_find_interface, get rid of BKL
>> >>
>> >> ? ? This removes the private hiddev_table in the usbhid
>> >> ? ? driver and changes it to use usb_find_interface
>> >> ? ? instead.
>> >>
>> >> ? ? The advantage is that we can avoid the race between
>> >> ? ? usb_register_dev and usb_open and no longer need the
>> >> ? ? big kernel lock.
>> >>
>> >> ? ? This doesn't introduce race condition -- the intf pointer could be
>> >> ? ? invalidated only in hiddev_disconnect() through usb_deregister_dev(),
>> >> ? ? but that will block on minor_rwsem and not actually remove the device
>> >> ? ? until usb_open().
>> >> -----------------------
>> >>
>> >> Despite mentioned issues with network (occurring every time after
>> >> unsuccessful suspend > hard restart) the bisecting turned up a commit
>> >> which isn't rather related to network. But I can confirm that
>> >> reverting only this one commit in 2.6.36-rc1
>> >> [da5cabf80e2433131bf0ed8993abc0f7ea618c73] fixes the whole problem -
>> >> suspend works OK and I have no issues with my network after waking the
>> >> system up, neither after re-booting from the same or any other kernel.
>> >>
>> >> Please let me know what further details should I provide. Any logs,
>> >> hardware specifications? I've also made a brief log recording the
>> >> bisect if anyone would need it.
>> >
>> > Well, we should let the HID maintainer know about the problem (CC added).
>>
>> Thanks Rafael,
>> Before I opened this thread I only searched on
>> http://vger.kernel.org/vger-lists.html for any appropriate mailing
>> list but nothing linke linux-*hid* there. Now I've found out that
>> there is a list of all maintainers included in the kernel source. I'll
>> keep that in mind ..
>
> Yeah, MAINTAINERS file is always good to consult. Also CCing all people
> who have Signed-off-by or Acked-by the offending patch is usually a good
> idea.
Thanks for the tip.
> Could you please verify whether the patch below (which is currently queued
> in my tree and I am planning to push it to Linus soon, as it fixes memory
> corruption) fixes your issue? Thanks.
>
> commit 9c9e54a8df0be48aa359744f412377cc55c3b7d2
> Author: Jiri Kosina <[email protected]>
> Date: ? Fri Aug 13 12:19:45 2010 +0200
>
> ? ?HID: hiddev: fix memory corruption due to invalid intfdata
>
> ? ?Commit bd25f4dd6972755579d0 ("HID: hiddev: use usb_find_interface,
> ? ?get rid of BKL") introduced using of private intfdata in hiddev for
> ? ?purpose of storing hiddev pointer.
>
> ? ?This is a problem, because intf pointer is already being set to struct
> ? ?hid_device pointer by HID core. This obviously lead to memory corruptions
> ? ?at device disconnect time, such as
>
> ? ?WARNING: at lib/kobject.c:595 kobject_put+0x37/0x4b()
> ? ?kobject: '(null)' (ffff88011e9cd898): is not initialized, yet kobject_put() is being called.
>
> ? ?Convert hiddev into accessing hiddev through struct hid_device which is
> ? ?in intfdata already.
>
> ? ?Reported-and-tested-by: Markus Trippelsdorf <[email protected]>
> ? ?Reported-and-tested-by: Heinz Diehl <[email protected]>
> ? ?Reported-and-tested-by: Alan Ott <[email protected]>
> ? ?Signed-off-by: Jiri Kosina <[email protected]>
>
> diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
> index f285017..0a29c51 100644
> --- a/drivers/hid/usbhid/hiddev.c
> +++ b/drivers/hid/usbhid/hiddev.c
> @@ -266,13 +266,15 @@ static int hiddev_open(struct inode *inode, struct file *file)
> ?{
> ? ? ? ?struct hiddev_list *list;
> ? ? ? ?struct usb_interface *intf;
> + ? ? ? struct hid_device *hid;
> ? ? ? ?struct hiddev *hiddev;
> ? ? ? ?int res;
>
> ? ? ? ?intf = usb_find_interface(&hiddev_driver, iminor(inode));
> ? ? ? ?if (!intf)
> ? ? ? ? ? ? ? ?return -ENODEV;
> - ? ? ? hiddev = usb_get_intfdata(intf);
> + ? ? ? hid = usb_get_intfdata(intf);
> + ? ? ? hiddev = hid->hiddev;
>
> ? ? ? ?if (!(list = kzalloc(sizeof(struct hiddev_list), GFP_KERNEL)))
> ? ? ? ? ? ? ? ?return -ENOMEM;
> @@ -890,7 +892,6 @@ int hiddev_connect(struct hid_device *hid, unsigned int force)
> ? ? ? ?hid->hiddev = hiddev;
> ? ? ? ?hiddev->hid = hid;
> ? ? ? ?hiddev->exist = 1;
> - ? ? ? usb_set_intfdata(usbhid->intf, usbhid);
> ? ? ? ?retval = usb_register_dev(usbhid->intf, &hiddev_class);
> ? ? ? ?if (retval) {
> ? ? ? ? ? ? ? ?err_hid("Not able to get a minor for this device.");
>
> --
> Jiri Kosina
> SUSE Labs, Novell Inc.
>
Yes. That patch fixes my problem too. Tested on top of the current
tree [2.6.36-rc1-00051-g3b89f56]. Thanks!
One little thing though. There is a message about 2 lines offset when
patching. Succeeded anyway but I've never got such messages before so
I thought it would be better to mention it:
patch -p1 <../patches/hid.git-9c9e54a8df0be48aa359744f412377cc55c3b7d2.patch
patching file drivers/hid/usbhid/hiddev.c
Hunk #2 succeeded at 890 (offset -2 lines).
I downloaded the patch from
http://git.kernel.org/?p=linux/kernel/git/jikos/hid.git;a=commitdiff;h=9c9e54a8df0be48aa359744f412377cc55c3b7d2
to make sure I had the original indentations etc. preserved (Gmail
tends to mess up with lines).
--
Regards
Tomasz B.
On Wed, 18 Aug 2010, trapDoor wrote:
> > Could you please verify whether the patch below (which is currently queued
> > in my tree and I am planning to push it to Linus soon, as it fixes memory
> > corruption) fixes your issue? Thanks.
> >
> > commit 9c9e54a8df0be48aa359744f412377cc55c3b7d2
> > Author: Jiri Kosina <[email protected]>
> > Date: ? Fri Aug 13 12:19:45 2010 +0200
> >
> > ? ?HID: hiddev: fix memory corruption due to invalid intfdata
> >
> > ? ?Commit bd25f4dd6972755579d0 ("HID: hiddev: use usb_find_interface,
> > ? ?get rid of BKL") introduced using of private intfdata in hiddev for
> > ? ?purpose of storing hiddev pointer.
> >
> > ? ?This is a problem, because intf pointer is already being set to struct
> > ? ?hid_device pointer by HID core. This obviously lead to memory corruptions
> > ? ?at device disconnect time, such as
> >
> > ? ?WARNING: at lib/kobject.c:595 kobject_put+0x37/0x4b()
> > ? ?kobject: '(null)' (ffff88011e9cd898): is not initialized, yet kobject_put() is being called.
> >
> > ? ?Convert hiddev into accessing hiddev through struct hid_device which is
> > ? ?in intfdata already.
> >
> > ? ?Reported-and-tested-by: Markus Trippelsdorf <[email protected]>
> > ? ?Reported-and-tested-by: Heinz Diehl <[email protected]>
> > ? ?Reported-and-tested-by: Alan Ott <[email protected]>
> > ? ?Signed-off-by: Jiri Kosina <[email protected]>
> >
> > diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
> > index f285017..0a29c51 100644
> > --- a/drivers/hid/usbhid/hiddev.c
> > +++ b/drivers/hid/usbhid/hiddev.c
> > @@ -266,13 +266,15 @@ static int hiddev_open(struct inode *inode, struct file *file)
> > ?{
> > ? ? ? ?struct hiddev_list *list;
> > ? ? ? ?struct usb_interface *intf;
> > + ? ? ? struct hid_device *hid;
> > ? ? ? ?struct hiddev *hiddev;
> > ? ? ? ?int res;
> >
> > ? ? ? ?intf = usb_find_interface(&hiddev_driver, iminor(inode));
> > ? ? ? ?if (!intf)
> > ? ? ? ? ? ? ? ?return -ENODEV;
> > - ? ? ? hiddev = usb_get_intfdata(intf);
> > + ? ? ? hid = usb_get_intfdata(intf);
> > + ? ? ? hiddev = hid->hiddev;
> >
> > ? ? ? ?if (!(list = kzalloc(sizeof(struct hiddev_list), GFP_KERNEL)))
> > ? ? ? ? ? ? ? ?return -ENOMEM;
> > @@ -890,7 +892,6 @@ int hiddev_connect(struct hid_device *hid, unsigned int force)
> > ? ? ? ?hid->hiddev = hiddev;
> > ? ? ? ?hiddev->hid = hid;
> > ? ? ? ?hiddev->exist = 1;
> > - ? ? ? usb_set_intfdata(usbhid->intf, usbhid);
> > ? ? ? ?retval = usb_register_dev(usbhid->intf, &hiddev_class);
> > ? ? ? ?if (retval) {
> > ? ? ? ? ? ? ? ?err_hid("Not able to get a minor for this device.");
> >
>
> Yes. That patch fixes my problem too. Tested on top of the current
> tree [2.6.36-rc1-00051-g3b89f56]. Thanks!
Thanks a lot for prompt testing.
> One little thing though. There is a message about 2 lines offset when
> patching. Succeeded anyway but I've never got such messages before so
> I thought it would be better to mention it:
>
> patch -p1 <../patches/hid.git-9c9e54a8df0be48aa359744f412377cc55c3b7d2.patch
> patching file drivers/hid/usbhid/hiddev.c
> Hunk #2 succeeded at 890 (offset -2 lines).
>
> I downloaded the patch from
> http://git.kernel.org/?p=linux/kernel/git/jikos/hid.git;a=commitdiff;h=9c9e54a8df0be48aa359744f412377cc55c3b7d2
> to make sure I had the original indentations etc. preserved (Gmail
> tends to mess up with lines).
Yeah, that's fine. It just means that the patch has been generated on a
slightly different version of the file, which makes the line numbers be
off a bit, but patch, git and friends can cope with this easily.
--
Jiri Kosina
SUSE Labs, Novell Inc.
On Wed, Aug 18, 2010 at 9:46 AM, Jiri Kosina <[email protected]> wrote:
> On Wed, 18 Aug 2010, trapDoor wrote:
>
>> > Could you please verify whether the patch below (which is currently queued
>> > in my tree and I am planning to push it to Linus soon, as it fixes memory
>> > corruption) fixes your issue? Thanks.
>> >
>> > commit 9c9e54a8df0be48aa359744f412377cc55c3b7d2
>> > Author: Jiri Kosina <[email protected]>
>> > Date: ? Fri Aug 13 12:19:45 2010 +0200
>> >
>> > ? ?HID: hiddev: fix memory corruption due to invalid intfdata
>> >
>> > ? ?Commit bd25f4dd6972755579d0 ("HID: hiddev: use usb_find_interface,
>> > ? ?get rid of BKL") introduced using of private intfdata in hiddev for
>> > ? ?purpose of storing hiddev pointer.
>> >
>> > ? ?This is a problem, because intf pointer is already being set to struct
>> > ? ?hid_device pointer by HID core. This obviously lead to memory corruptions
>> > ? ?at device disconnect time, such as
>> >
>> > ? ?WARNING: at lib/kobject.c:595 kobject_put+0x37/0x4b()
>> > ? ?kobject: '(null)' (ffff88011e9cd898): is not initialized, yet kobject_put() is being called.
>> >
>> > ? ?Convert hiddev into accessing hiddev through struct hid_device which is
>> > ? ?in intfdata already.
>> >
>> > ? ?Reported-and-tested-by: Markus Trippelsdorf <[email protected]>
>> > ? ?Reported-and-tested-by: Heinz Diehl <[email protected]>
>> > ? ?Reported-and-tested-by: Alan Ott <[email protected]>
>> > ? ?Signed-off-by: Jiri Kosina <[email protected]>
>> >
>> > diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
>> > index f285017..0a29c51 100644
>> > --- a/drivers/hid/usbhid/hiddev.c
>> > +++ b/drivers/hid/usbhid/hiddev.c
>> > @@ -266,13 +266,15 @@ static int hiddev_open(struct inode *inode, struct file *file)
>> > ?{
>> > ? ? ? ?struct hiddev_list *list;
>> > ? ? ? ?struct usb_interface *intf;
>> > + ? ? ? struct hid_device *hid;
>> > ? ? ? ?struct hiddev *hiddev;
>> > ? ? ? ?int res;
>> >
>> > ? ? ? ?intf = usb_find_interface(&hiddev_driver, iminor(inode));
>> > ? ? ? ?if (!intf)
>> > ? ? ? ? ? ? ? ?return -ENODEV;
>> > - ? ? ? hiddev = usb_get_intfdata(intf);
>> > + ? ? ? hid = usb_get_intfdata(intf);
>> > + ? ? ? hiddev = hid->hiddev;
>> >
>> > ? ? ? ?if (!(list = kzalloc(sizeof(struct hiddev_list), GFP_KERNEL)))
>> > ? ? ? ? ? ? ? ?return -ENOMEM;
>> > @@ -890,7 +892,6 @@ int hiddev_connect(struct hid_device *hid, unsigned int force)
>> > ? ? ? ?hid->hiddev = hiddev;
>> > ? ? ? ?hiddev->hid = hid;
>> > ? ? ? ?hiddev->exist = 1;
>> > - ? ? ? usb_set_intfdata(usbhid->intf, usbhid);
>> > ? ? ? ?retval = usb_register_dev(usbhid->intf, &hiddev_class);
>> > ? ? ? ?if (retval) {
>> > ? ? ? ? ? ? ? ?err_hid("Not able to get a minor for this device.");
>> >
>>
>> Yes. That patch fixes my problem too. Tested on top of the current
>> tree [2.6.36-rc1-00051-g3b89f56]. Thanks!
>
> Thanks a lot for prompt testing.
>
>> One little thing though. There is a message about 2 lines offset when
>> patching. Succeeded anyway but I've never got such messages before so
>> I thought it would be better to mention it:
>>
>> patch -p1 <../patches/hid.git-9c9e54a8df0be48aa359744f412377cc55c3b7d2.patch
>> patching file drivers/hid/usbhid/hiddev.c
>> Hunk #2 succeeded at 890 (offset -2 lines).
>>
>> I downloaded the patch from
>> http://git.kernel.org/?p=linux/kernel/git/jikos/hid.git;a=commitdiff;h=9c9e54a8df0be48aa359744f412377cc55c3b7d2
>> to make sure I had the original indentations etc. preserved (Gmail
>> tends to mess up with lines).
>
> Yeah, that's fine. It just means that the patch has been generated on a
> slightly different version of the file, which makes the line numbers be
> off a bit, but patch, git and friends can cope with this easily.
>
Thanks for confirming.
Andrea, Gabriel
Would you please check if Jiri's patch solves the problem on your end
as well? Just to make 100% sure we can close related bugzilla call.
--
Regards
Tomasz B.
2010/8/18 trapDoor <[email protected]>:
> Andrea, Gabriel
> Would you please check if Jiri's patch solves the problem on your end
> as well? Just to make 100% sure we can close related bugzilla call.
I hope to try it tomorrow.
Thanks a lot for your work,
Andrea
2010/8/18 trapDoor <[email protected]>:
> Andrea, Gabriel
> Would you please check if Jiri's patch solves the problem on your end
> as well? Just to make 100% sure we can close related bugzilla call.
Well, I made the test, with ~10 cycle hibernation/resume.
Everything seems goes right.
Thanks a lot for your work,
Andrea
2010/8/18 Andrea Gelmini <[email protected]>
>
> 2010/8/18 trapDoor <[email protected]>:
> > Andrea, Gabriel
> > Would you please check if Jiri's patch solves the problem on your end
> > as well? Just to make 100% sure we can close related bugzilla call.
>
> Well, I made the test, with ~10 cycle hibernation/resume.
> Everything seems goes right.
>
Does work fine here also ..
Regards,
Gabriel
On Wed, 18 Aug 2010, Gabriel Craciunescu wrote:
> > > Andrea, Gabriel
> > > Would you please check if Jiri's patch solves the problem on your end
> > > as well? Just to make 100% sure we can close related bugzilla call.
> >
> > Well, I made the test, with ~10 cycle hibernation/resume.
> > Everything seems goes right.
> >
>
> Does work fine here also ..
Thanks everyone for confirming. The fix is now in Linus' tree and thus
will be in -rc2.
--
Jiri Kosina
SUSE Labs, Novell Inc.
On Thu, Aug 19, 2010 at 8:40 AM, Jiri Kosina <[email protected]> wrote:
> On Wed, 18 Aug 2010, Gabriel Craciunescu wrote:
>
>> > > Andrea, Gabriel
>> > > Would you please check if Jiri's patch solves the problem on your end
>> > > as well? Just to make 100% sure we can close related bugzilla call.
>> >
>> > Well, I made the test, with ~10 cycle hibernation/resume.
>> > Everything seems goes right.
>> >
>>
>> Does work fine here also ..
>
> Thanks everyone for confirming. The fix is now in Linus' tree and thus
> will be in -rc2.
>
> --
> Jiri Kosina
> SUSE Labs, Novell Inc.
>
I saw Linus merged your HID pull request yesterday. I've got it now
and can confirm again that everything works OK.
--
Thanks
Tomasz B.