linux-kernel - Re: [ 67/75] ipw2200: Fix race condition in the command completion acknowledge

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]

Message-ID: <1336436053.8274.333.camel@deadeye>
Date:	Tue, 08 May 2012 01:14:13 +0100
From:	Ben Hutchings <ben@...adent.org.uk>
To:	Stanislav Yakovlev <stas.yakovlev@...il.com>
Cc:	"John W. Linville" <linville@...driver.com>,
	linux-kernel@...r.kernel.org, stable@...r.kernel.org,
	torvalds@...ux-foundation.org, akpm@...ux-foundation.org,
	alan@...rguk.ukuu.org.uk, Greg KH <gregkh@...uxfoundation.org>
Subject: Re: [ 67/75] ipw2200: Fix race condition in the command completion
 acknowledge

On Mon, 2012-05-07 at 16:45 -0700, Stanislav Yakovlev wrote:
> On 7 May 2012 07:37, Ben Hutchings <ben@...adent.org.uk> wrote:
> > On Fri, 2012-05-04 at 13:43 -0700, Greg KH wrote:
> >> 3.3-stable review patch.  If anyone has any objections, please let me know.
> >>
> >> ------------------
> >>
> >> From: Stanislav Yakovlev <stas.yakovlev@...il.com>
> >>
> >> commit dd447319895d0c0af423e483d9b63f84f3f8869a upstream.
> >>
> >> Driver incorrectly validates command completion: instead of waiting
> >> for a command to be acknowledged it continues execution.  Most of the
> >> time driver gets acknowledge of the command completion in a tasklet
> >> before it executes the next one. But sometimes it sends the next
> >> command before it gets acknowledge for the previous one. In such a
> >> case one of the following error messages appear in the log:
> > [...]
> >> +     now = jiffies;
> >> +     end = now + HOST_COMPLETE_TIMEOUT;
> >> +again:
> >>       rc = wait_event_interruptible_timeout(priv->wait_command_queue,
> >>                                             !(priv->
> >>                                               status & STATUS_HCMD_ACTIVE),
> >> -                                           HOST_COMPLETE_TIMEOUT);
> >> +                                           end - now);
> >> +     if (rc < 0) {
> >> +             now = jiffies;
> >> +             if (time_before(now, end))
> >> +                     goto again;
> >> +             rc = 0;
> >> +     }
> > [...]
> >
> > If you don't want the wait to be interrupted, use wait_event_timeout()
> > instead of this ridiculous loop!
> 
> Usually "modprobe ipw2200" takes less than a second, and after
> switching to wait_event_timeout it will take more than 10 seconds. We
> can not decrease the waiting timeout because we want to keep it big
> enough for the worst case(when the firmware did not reply).
> 
> There is a relevant discussion on lklm:
> https://lkml.org/lkml/2008/9/25/2
> https://lkml.org/lkml/2008/9/26/194

That's irrelevant: you aren't letting userland handle the signal here,
so you should use wait_event_timeout() and no loop.

Ben.

-- 
Ben Hutchings
Life is what happens to you while you're busy making other plans.
                                                               - John Lennon

Download attachment "signature.asc" of type "application/pgp-signature" (829 bytes)