netdev - Re: [PATCH] 3c59x: fix incorrect use of spin_lock

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] [day] [month] [year] [list]

Message-ID: <20131024224447.GA27710@electric-eye.fr.zoreil.com>
Date:	Fri, 25 Oct 2013 00:44:47 +0200
From:	Francois Romieu <romieu@...zoreil.com>
To:	Mikulas Patocka <mpatocka@...hat.com>
Cc:	David Miller <davem@...emloft.net>,
	klassert@...hematik.tu-chemnitz.de, netdev@...r.kernel.org
Subject: Re: [PATCH] 3c59x: fix incorrect use of spin_lock_bh in interrupts

Mikulas Patocka <mpatocka@...hat.com> :
> On Tue, 22 Oct 2013, David Miller wrote:
[...]
> > vortex_down() does a lot of other things which are really dangerous
> > from an interrupt handler, such as del_timer_sync().
> > 
> > The real fix for this bug is to defer the vortex_error() work into
> > a workqueue, and thus process context, like every other driver does.
> 
> That del_timer_sync() could be skipped - if we reset the card, we don't 
> need to reinitialize the times. Do you see anything else there that 
> prevents the functions vortex_down and vortex_up from being called from an 
> interrupt?

Up to 1s busy wait loop in issue_and_wait.

window_{read / write} deadlock with vortex_tx_timeout.

Error recovery needs to be robust and you'd rather recover in a context
where you can be a real pig (MDIO usually is).

It is a common pattern in linux kernel ethernet drivers to reduce the irq
handler to a bare minimum and move real work to softirq (NAPI) context,
especially for rx and tx traffic, then to workqueues for rare or error
related stuff. Window locking may limit the opportunity for fast path
locks removal but the driver would -imho- still end simpler.

-- 
Ueimor
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html