netdev - Re: [PATCH 2/2] mv643xx_eth: fix race in trasmit path.

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: <1308590350.2658.6.camel@edumazet-laptop>
Date:	Mon, 20 Jun 2011 19:19:10 +0200
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Lennert Buytenhek <buytenh@...tstofly.org>
Cc:	Richard Cochran <richardcochran@...il.com>, netdev@...r.kernel.org,
	David Miller <davem@...emloft.net>, stable@...nel.org
Subject: Re: [PATCH 2/2] mv643xx_eth: fix race in trasmit path.

Le lundi 20 juin 2011 à 18:33 +0200, Lennert Buytenhek a écrit :
> On Mon, Jun 20, 2011 at 09:48:07AM +0200, Richard Cochran wrote:
> 
> > Because the socket buffer is freed in the completion interrupt, it
> > is not safe to access it after submitting it to the hardware.
> 
> Maybe I'm missing something here, but mv643xx_eth TX reclaim is done
> from NAPI poll, under __netif_tx_lock(), while mv643xx_eth_xmit() also
> runs under __netif_tx_lock().

See my previous answer. Its true this driver _currently_ holds tx queue
lock in its TX completion. But that might/should change.

Goal is to make tx completion not use tx queue lock in fast path, like
its done in tg3, bnx2, bnx2x ... and other recent drivers.

Its obviously correct to move skb->len access in start_xmit() before
starting the IO, even if not a bug fix, it makes all drivers behave the
same : When reviewing them, its easier not to worry about these possible
use after free.

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