linux-kernel - Re: [PATCH] net: wireless: airo semaphore to mutex

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 for Android: free password hash cracker in your pocket

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

Date:	Tue, 27 May 2008 09:48:28 -0400
From:	Dan Williams <dcbw@...hat.com>
To:	Matthew Wilcox <matthew@....cx>
Cc:	Daniel Walker <dwalker@...sta.com>, linville@...driver.com,
	rusty@...tcorp.com.au, mingo@...e.hu, linux-kernel@...r.kernel.org,
	linux-wireless@...r.kernel.org, arjan@...radead.org
Subject: Re: [PATCH] net: wireless: airo semaphore to mutex

On Tue, 2008-05-27 at 07:13 -0600, Matthew Wilcox wrote:
> On Thu, May 22, 2008 at 04:21:03PM -0700, Daniel Walker wrote:
> > Signed-off-by: Daniel Walker <dwalker@...sta.com>
> 
> Reviewing the driver, this really doesn't feel like a classical lock.
> It feels very much ad-hoc, with most references to ->sem being
> unbalanced.

It should be a classical lock actually.  There shouldn't be any case
where semaphore behavior is used or needed.

> The driver has a thread (airo_thread) and it seems to be trying to
> synchronise against both the interrupt handler, and other threads
> (calling eg ->get_stats).

Right, though it's important to note that anything coming from WEXT will
already be under the rtnl lock and should already be serialized, so we
don't need to protect any of the WEXT handlers _from themselves_, just
from the main thread and the interrupt handler.

> I think it really wants to be using spin_lock_irqsave() instead of a
> mutex.  I'd want to study the driver in a lot more detail before making
> that kind of change though.

Yeah, that might be the right way to go.  If you want to jump into the
driver, feel quite free, but I don't think you need to go into too much
detail with it.  Your overall behavior analysis is right on the mark
already.

Dan

--
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/