linux-kernel - Re: [PATCH] Export current_is

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: <Pine.LNX.4.64N.0612061210530.29000@blysk.ds.pg.gda.pl>
Date:	Wed, 6 Dec 2006 12:31:15 +0000 (GMT)
From:	"Maciej W. Rozycki" <macro@...ux-mips.org>
To:	Andy Fleming <afleming@...escale.com>
cc:	Andrew Morton <akpm@...l.org>,
	Ben Collins <ben.collins@...ntu.com>,
	linux-kernel@...r.kernel.org, Linus Torvalds <torvalds@...l.org>,
	Jeff Garzik <jeff@...zik.org>
Subject: Re: [PATCH] Export current_is_keventd() for libphy

On Tue, 5 Dec 2006, Andy Fleming wrote:

> We need to make sure there are no more pending phy_change() invocations in the
> work queue, this is true, however I'm not convinced that this avoids the
> problem.  And now that I come back to this email after Linus's response, let
> me add that I agree with his suggestion.  I still don't think it solves the
> original problem, though.  Unless I'm missing something, Maciej's suggested
> fix (having the driver invoke phy_stop_interrupts() from a work queue) doesn't
> stop the race:
> 
> * Schedule stop_interrupts and freeing of memory.
> * interrupt occurs, and schedules phy_change
> * work_queue triggers, and stop_interrupts is invoked.  It doesn't call
> flush_scheduled_work, because it's being called from keventd.
> * The invoker of stop_interrupts (presumably some function in the driver)
> frees up memory, including the phy_device.
> * phy_change is invoked() from the work queue, and starts accessing freed
> memory

 This is not going to happen with my other changes to the file applied.  
The reason is at the time phy_stop_interrupts() is called phy_stop() has 
already run and switched the state of the PHY being handled to PHY_HALTED.  
As a result any subsequent calls to phy_interrupt() that might have 
happened after phy_stop() have not scheduled calls to phy_change() for 
this PHY as will not any that may happen up until free_irq() have 
unregistered the interrupt for the PHY.

> I suggested this, mostly so that drivers wouldn't have to be aware of this.
> But I'm not quite sure what happens when you unload a module.  Does some stuff
> stay behind if needed?  If you unload the ethernet driver, that will usually
> remove the bus controller for the PHY, which would prevent any scheduled code
> from accessing the PHY.

 Hmm, I am unsure if there is anything that would ensure flushing of the 
queue after its stop() call has finished and before a driver is removed 
(its module_exit() call is invoked), probably nothing, and that is why I 
put explicit flush_scheduled_work() in sb1250-mac.c:sbmac_remove() and the 
driver's open() call checks whether a possible previous instance of the 
structure used by phy_change() have not been freed yet.  There may be a 
cleaner way of doing it, but I will have to think about it.

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