netdev - Re: [PATCH net] net: dsa: sja1105: Fix double delivery of TX timestamps to socket error queue

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] [day] [month] [year] [list]

Date:   Fri, 27 Dec 2019 09:39:50 -0800
From:   Richard Cochran <richardcochran@...il.com>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     "Keller, Jacob E" <jacob.e.keller@...el.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "jakub.kicinski@...ronome.com" <jakub.kicinski@...ronome.com>,
        "f.fainelli@...il.com" <f.fainelli@...il.com>,
        "vivien.didelot@...il.com" <vivien.didelot@...il.com>,
        "andrew@...n.ch" <andrew@...n.ch>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net] net: dsa: sja1105: Fix double delivery of TX
 timestamps to socket error queue

On Fri, Dec 27, 2019 at 05:30:09PM +0200, Vladimir Oltean wrote:
> But in the case that I _do_ care about, it _is_ caused by DSA. The
> gianfar driver is not expecting anybody else to set SKBTX_IN_PROGRESS,
> which in itself is not illegal, whether or not you argue that it is
> needed or not.

Ok, so I've reviewed the whole SKBTX_IN_PROGRESS thing again, and I'm
starting to see it your way.  (Or maybe not ;^)

The purpose of SKBTX_IN_PROGRESS is to prevent a SW transmit time
stamp from being delivered (unless SOF_TIMESTAMPING_OPT_TX_SWHW is set
on the socket).

So DSA drivers (and PHY drivers) should indeed set this flag.

However, most (or all?) MAC drivers do not expect anyone else to have
set this flag.

In the case of DSA, the DSA driver has the chance to set the flag
first, before the skb is passed to the MAC driver.  (PHY drivers don't
have first dibs, but let's concentrate on DSA for now.)

So, you could introduce a new rule that MAC drivers must check to see
if the flag is already set, and if so leave it alone.

MAC drivers should in any case respect their current SIOCSHWTSTAMP
configuration and not attempt time stamping when it is not enabled.

So I'm afraid that, as I've said before, you'll have to patch the MAC
drivers one by.  It will be lots of work.

But if and when you submit patches for the MAC drivers, please
remember that DSA time stamping is the new kid on the block, and
yelling that the MAC drivers are broken won't make you any friends.

Thanks,
Richard