netdev - Re: [PCNET32] Lock solid with netconsole

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:	Mon, 28 May 2007 22:51:44 +0200
From:	tsbogend@...ha.franken.de (Thomas Bogendoerfer)
To:	Lennart Sorensen <lsorense@...lub.uwaterloo.ca>
Cc:	Emmanuel Fust? <emmanuel.fuste@...oste.net>,
	romieu <romieu@...zoreil.com>, pcnet32 <pcnet32@...izon.net>,
	netdev <netdev@...r.kernel.org>
Subject: Re: [PCNET32] Lock solid with netconsole

On Mon, May 28, 2007 at 02:31:48PM -0400, Lennart Sorensen wrote:
> On Mon, May 28, 2007 at 05:25:51PM +0200, Emmanuel Fust? wrote:
> > > Any difference if you disable the debug messages in the pcnet32
> > > driver and you apply the patch below ?
> > > 
> > > diff --git a/drivers/net/pcnet32.c b/drivers/net/pcnet32.c
> > > index 9c171a7..be4513f 100644
> > > --- a/drivers/net/pcnet32.c
> > > +++ b/drivers/net/pcnet32.c
> > > @@ -2556,11 +2556,12 @@ pcnet32_interrupt(int irq, void *dev_id)
> > >  	unsigned long ioaddr;
> > >  	u16 csr0;
> > >  	int boguscnt = max_interrupt_work;
> > > +	unsigned long flags;
> > >  
> > >  	ioaddr = dev->base_addr;
> > >  	lp = netdev_priv(dev);
> > >  
> > > -	spin_lock(&lp->lock);
> > > +	spin_lock_irqsave(&lp->lock, flags);
> > >  
> > >  	csr0 = lp->a.read_csr(ioaddr, CSR0);
> > >  	while ((csr0 & 0x8f00) && --boguscnt >= 0) {
> > > @@ -2632,7 +2633,7 @@ pcnet32_interrupt(int irq, void *dev_id)
> > >  		printk(KERN_DEBUG "%s: exiting interrupt, csr0=%#4.4x.\n",
> > >  		       dev->name, lp->a.read_csr(ioaddr, CSR0));
> > >  
> > > -	spin_unlock(&lp->lock);
> > > +	spin_unlock_irqrestore(&lp->lock, flags);
> > >  
> > >  	return IRQ_HANDLED;
> > >  }
> > > 
> > Hi,
> > Tested under very high console activity and it no longer freeze.
> 
> Hmm, I have been seeing lockups too and asked about doing something
> almost exactly the same as this recently, but was told that it shouldn't
> need irqs disabled at this point.  Well if it makes netconsole more

for normal interrupt delivery it doesn't matter, because there shouldn't
be any more interrupts coming in at that point. But netconsole uses
pcnet32_interrupt for polling the chip. So if during service of a
a real interrupt a polled pcnet32_interrupt call is done, the machine
will deadlock.

Using spin_lock_irqsave() is probably the only race free solution,
when using NET_POLL.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessary a
good idea.                                                [ RFC1925, 2.3 ]
-
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