linux-kernel - Re: [PATCH 2/2] net: ethernet: stmmac: remove private tx queue 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20161207205942.GB2250@amd>
Date:   Wed, 7 Dec 2016 21:59:42 +0100
From:   Pavel Machek <pavel@....cz>
To:     Lino Sanfilippo <LinoSanfilippo@....de>
Cc:     bh74.an@...sung.com, ks.giri@...sung.com, vipul.pandya@...sung.com,
        peppe.cavallaro@...com, alexandre.torgue@...com,
        davem@...emloft.net, linux-kernel@...r.kernel.org,
        netdev@...r.kernel.org
Subject: Re: [PATCH 2/2] net: ethernet: stmmac: remove private tx queue lock

Hi!

> The driver uses a private lock for synchronization between the xmit
> function and the xmit completion handler, but since the NETIF_F_LLTX flag
> is not set, the xmit function is also called with the xmit_lock held.
> 
> On the other hand the xmit completion handler first takes the private lock
> and (in case that the tx queue has been stopped) the xmit_lock, leading to
> a reverse locking order and the potential danger of a deadlock.
> 
> Fix this by removing the private lock completely and synchronizing the xmit
> function and completion handler solely by means of the xmit_lock. By doing
> this remove also the now unnecessary double check for a stopped tx queue.
> 
> Signed-off-by: Lino Sanfilippo <LinoSanfilippo@....de>

Oops, sorry no, that broke the driver after a while:

(So please ignore my tested-by:)

root@...abuibui:/data/tmp/udpt# ./udp-test raw 10.0.0.6 1234 1000 100
30
Sending 100 packets (1000b each) at an interval of 30ms, expected data
rate:3333333b/s (3373333b/s incl udp overhead)
[   30.948626] socfpga-dwmac ff702000.ethernet eth0: Link is Up -
100Mbps/Full - flow control rx/tx
[   31.076064] Link is Up - 100/Full
[   32.979526] random: crng init done
[  262.244030] ------------[ cut here ]------------
[  262.248669] WARNING: CPU: 0 PID: 0 at net/sched/sch_generic.c:316
dev_watchdog+0x254/0x26c
[  262.256916] NETDEV WATCHDOG: eth0 (socfpga-dwmac): transmit queue 0
timed out
[  262.264028] Modules linked in:
[  262.267102] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
4.9.0-rc7-118095-g2d70d9b-dirty #339
[  262.275328] Hardware name: Altera SOCFPGA
[  262.279352] [<8010f758>] (unwind_backtrace) from [<8010affc>]
(show_stack+0x10/0x14)

									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Download attachment "signature.asc" of type "application/pgp-signature" (182 bytes)