linux-kernel - Re: 2.6.39-rc2 regression: X201s fails to resume b77dcf8460ae57d4eb9fd3633eb4f97b8fb20716

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 12 Apr 2011 00:16:32 +0200 (CEST)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Marcel Holtmann <marcel@...tmann.org>
cc:	Vinicius Costa Gomes <vinicius.gomes@...nbossa.org>,
	Keith Packard <keithp@...thp.com>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	linux-bluetooth@...r.kernel.org
Subject: Re: 2.6.39-rc2 regression: X201s fails to resume
 b77dcf8460ae57d4eb9fd3633eb4f97b8fb20716

On Mon, 11 Apr 2011, Marcel Holtmann wrote:

> Hi Thomas,
> 
> > > > > Can the bluetooth folks please have a look at that ASAP? The obvious
> > > > > fast fix for Linus tree is to revert the second hunk for now, but this
> > > > > needs to be fixed proper.
> > > > 
> > > > Who will submit this patch? I'd rather have your name on it so that
> > > > people come complain at you...
> > > 
> > > I took a shot at it and just sent a patch (also attached for convenience) 
> > > that should solve the problem.
> > 
> > Aaarg. No. That patch reverts both hunks.
> > 
> > --- a/net/bluetooth/hci_core.c
> > +++ b/net/bluetooth/hci_core.c
> > @@ -586,9 +586,6 @@ static int hci_dev_do_close(struct hci_dev *hdev)
> >  	hci_req_cancel(hdev, ENODEV);
> >  	hci_req_lock(hdev);
> >  
> > -	/* Stop timer, it might be running */
> > -	del_timer_sync(&hdev->cmd_timer);
> > -
> >  	if (!test_and_clear_bit(HCI_UP, &hdev->flags)) {
> >  		hci_req_unlock(hdev);
> >  		return 0;
> > 
> > As I said before you need that first hunk to stay for the case where
> > there is no device up and you return via the !HCI_UP check. You just
> > moved back to the state before as the stupid timer is active for
> > whatever reason even when HCI_UP is not set.
> 
> if I read this right then we have the case that we arm this timer for no
> real reason. A device in !HCI_UP should have nothing running. Certainly
> not the cmd_timer since it will never process any commands.
> 
> According to Gustavo, the problem is really in the hci_reset logic were
> we arm the timer even when shutting down the device.

The reason why the original patch was sent is, that the timer was
running when the thing went out via the !HCI_UP path, which caused the
whole thing to explode in the first place. I had no time to figure out
why, but moving the del_timer_sync above the
if (!test_and_clear_bit(HCI_UP, &hdev->flags)) solved it.

Thanks,

	tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/