netdev - Re: [Kgdb-bugreport] [PATCH] 8139too: harden against TX ring overflow

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: <46165BD7.2090605@ru.mvista.com>
Date:	Fri, 06 Apr 2007 18:40:23 +0400
From:	Sergei Shtylyov <sshtylyov@...mvista.com>
To:	"Amit S. Kale" <amitkale@...syssoft.com>
CC:	kgdb-bugreport@...ts.sourceforge.net,
	Herbert Xu <herbert@...dor.apana.org.au>,
	netdev@...r.kernel.org, jgarzik@...ox.com, mhuth@...sta.com
Subject: Re: [Kgdb-bugreport] [PATCH] 8139too: harden against TX ring overflow

Hello.

Amit S. Kale wrote:

>>>This driver's 4-packet deep TX queue is too sensible to the "careless"
>>>callers ignoring its state (like netpoll in trapped mode), so add "queue
>>>full" check at the start of the hard_start_xmit() method (only under
>>>#ifndef RTL8139_NDEBUG, otherwise the queue will get stuck once dirty
>>>pointer gets out of sync); switch to using appropriate mnemonics for the
>>>return values while at it.

>>Could you please describe this netpoll scenario in more detail?
>>More importantly, why wouldn't we fix netpoll instead?

> We're trying to figure out a way of fixing netpoll. Don't know what the 
> solution is yet.

> Here is what happens: in KGDB we set netpoll trapped flag. This prevents 
> stopping and starting of a netdev queue. Interfaces that have a small ring 
> (8139) run into a problem because of this. When the ring goes full, it can't 
> stop the queue. This doesn't make sense since in absence of ring descriptors, 
> the device can't transmit any more packets. Sergie had posted one more patch 
> last week that lets us start and stop queues in trapped state.
> 
> This patch fixes the 8139 side behavior in this context.

    Not really -- the patch doesn't seem necessary in this context.  It's just 
an attempt to make the driver more robust against out-of-sync dirty pointer 
and posting packets to already stopped queue -- that check constituting the 
second part is still necessary in certain out-of-sync pointer situation where 
the queue may erroneously be woken up, i.e. with the difference between the 
dirty_tx and cur_tx is multiple of 4 by the end of loop (well, I guess that 
has happened only with netpoll in trapped mode so far).  Remember that all 
these checks are done only when the driver debugging is enabled (it *is* 
enabled by default though).

> -Amit

MBR, Sergei
-
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