Bug 16615

(In reply to comment #0)

Hello,

I have the same problem and was about to bisect the issue too.

I can also confirm that reverting bd25f4dd6972755579d0ea50d1a5ace2e9b00d1a makes suspend works again here.

Best Regards,

Gabriel C.

Thanks Gabriel,

I've got another thread for this on LKML. Would you like me to CC to my e-mails there?

Thanks,
Tomasz B.

Hi all,
   same problem here.
   I didn't bisect, and I can't test the revert 'til next week.

Thanks a lot,
Andrea

(In reply to comment #2)
> Thanks Gabriel,
> 
> I've got another thread for this on LKML. Would you like me to CC to my
> e-mails
> there?
> 

Sure just add me to CC there..

(In reply to comment #4)
> (In reply to comment #2)
> > Thanks Gabriel,
> > 
> > I've got another thread for this on LKML. Would you like me to CC to my
> e-mails
> > there?
> > 
> 
> Sure just add me to CC there..

OK. I will do next time.

Thanks.

Patch 9c9e54a8df0be48aa359744f412377cc55c3b7d2 fixes the problem on my end. Please refer to http://lkml.org/lkml/2010/8/18/92 for details.

Sent e-mail to Andrea and Gabriel asking for testing on their end.

Tomasz B.

Here is an e-mail from Jiri Kosing with a patch that fixes the problem. A copy was sent to LKML and the link in my previous comment refers to it but LKML seems to be down.

The patch can be found here:
http://git.kernel.org/?p=linux/kernel/git/jikos/hid.git;a=commitdiff;h=9c9e54a8df0be48aa359744f412377cc55c3b7d2

Andrea and Gabriel have confirmed that it fixed the problem on their end too.

---------- Forwarded message ----------

From: Jiri Kosina <jkosina@suse.cz>
Date: Wed, Aug 18, 2010 at 9:46 AM
Subject: Re: [REGRESSION, bisected] 2.6.36-rc1 - suspend issues
To: trapDoor <trapdoor6@gmail.com>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>, LKML <linux-kernel@vger.kernel.org>, arnd@arndb.de, osnix0@googlemail.com, andrea.gelmini@gelma.net, trapDoor <trapdoor6@googlemail.com>


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 <jkosina@suse.cz>
> > 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 <markus@trippelsdorf.de>
> >    Reported-and-tested-by: Heinz Diehl <htd@fritha.org>
> >    Reported-and-tested-by: Alan Ott <alan@signal11.us>
> >    Signed-off-by: Jiri Kosina <jkosina@suse.cz>
> >
> > 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.

---------- End of forwarded message ----------


Tomasz B.

And below is the final confirmation from Andrea and Gabriel. Therefore I'm closing this call.

---------- Forwarded message ----------
From: Gabriel Craciunescu <osnix0@googlemail.com>
Date: Wed, Aug 18, 2010 at 3:48 PM
Subject: Re: [REGRESSION, bisected] 2.6.36-rc1 - suspend issues
To: Andrea Gelmini <andrea.gelmini@gelma.net>
Cc: trapdoor6@gmail.com, Jiri Kosina <jkosina@suse.cz>, "Rafael J. Wysocki" <rjw@sisk.pl>, LKML <linux-kernel@vger.kernel.org>, arnd@arndb.de, trapDoor <trapdoor6@googlemail.com>


2010/8/18 Andrea Gelmini <andrea.gelmini@gelma.net>
>
> 2010/8/18 trapDoor <trapdoor6@gmail.com>:
> > 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

---------- End of forwarded message ----------

--
Tomasz B.