netdev - Re: [PATCH v2] ethernet:arc: Fix racing of TX ring buffer

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: <20160524190235.GA31006@electric-eye.fr.zoreil.com>
Date:	Tue, 24 May 2016 21:02:35 +0200
From:	Francois Romieu <romieu@...zoreil.com>
To:	Lino Sanfilippo <LinoSanfilippo@....de>
Cc:	Shuyu Wei <wsy2220@...il.com>, David Miller <davem@...emloft.net>,
	wxt@...k-chips.com, heiko@...ech.de,
	linux-rockchip@...ts.infradead.org, netdev@...r.kernel.org,
	al.kochet@...il.com
Subject: Re: [PATCH v2] ethernet:arc: Fix racing of TX ring buffer

Lino Sanfilippo <LinoSanfilippo@....de> :
[...]
> I dont agree here. A dma_wmb would have an effect to "data" and "info", yes,
> but it would have absolutely no effect to skb_tx_timestamp(), since there
> is no dma access involved here. In fact skb_tx_timestamp() could probably
> be even reordered to appear after the dma_wmb.
>
> Anyway, there is the wmb() directly after the assignment to "info". _This_
> barrier  should ensure that skb_tx_timestamp() (along with a flush of data
> and info to DMA) is executed before "txbd_curr" is advanced.
> This means that the corresponding skb cant be freed prematurely by tx_clean().

The concern here is about sending adequate PTP payload on the network.

skb_tx_timestamp() is used for network clock synchronization. Some extra
information must be transmitted. Be it through direct payload change
or through indirect control, it _is_ related to dma.

Several (most ?) skb_tx_timestamp() misuses blur the picture: CPU vs device
sync is of course way below the radar when the driver crashes because of
plain use-after-free skb_tx_timestamp() :o/

-- 
Ueimor