netdev - Re: [PATCH net-next v2 0/2] timestamp: control SOF_TIMESTAMPING_RX

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <66d0a0816d6ce_39197c29476@willemb.c.googlers.com.notmuch>
Date: Thu, 29 Aug 2024 12:23:29 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Jason Xing <kerneljasonxing@...il.com>, 
 Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc: davem@...emloft.net, 
 edumazet@...gle.com, 
 kuba@...nel.org, 
 pabeni@...hat.com, 
 dsahern@...nel.org, 
 willemb@...gle.com, 
 netdev@...r.kernel.org, 
 Jason Xing <kernelxing@...cent.com>
Subject: Re: [PATCH net-next v2 0/2] timestamp: control
 SOF_TIMESTAMPING_RX_SOFTWARE feature per socket

Jason Xing wrote:
> On Thu, Aug 29, 2024 at 10:14 PM Willem de Bruijn
> <willemdebruijn.kernel@...il.com> wrote:
> >
> > Jason Xing wrote:
> > > From: Jason Xing <kernelxing@...cent.com>
> > >
> > > Prior to this series, when one socket is set SOF_TIMESTAMPING_RX_SOFTWARE
> > > which measn the whole system turns on this button, other sockets that only
> > > have SOF_TIMESTAMPING_SOFTWARE will be affected and then print the rx
> > > timestamp information even without SOF_TIMESTAMPING_RX_SOFTWARE flag.
> > > In such a case, the rxtimestamp.c selftest surely fails, please see
> > > testcase 6.
> > >
> > > In a normal case, if we only set SOF_TIMESTAMPING_SOFTWARE flag, we
> > > can't get the rx timestamp because there is no path leading to turn on
> > > netstamp_needed_key button in net_enable_timestamp(). That is to say, if
> > > the user only sets SOF_TIMESTAMPING_SOFTWARE, we don't expect we are
> > > able to fetch the timestamp from the skb.
> >
> > I already happened to stumble upon a counterexample.
> >
> > The below code requests software timestamps, but does not set the
> > generate flag. I suspect because they assume a PTP daemon (sfptpd)
> > running that has already enabled that.
> 
> To be honest, I took a quick search through the whole onload program
> and then suspected the use of timestamp looks really weird.
> 
> 1. I searched the SOF_TIMESTAMPING_RX_SOFTWARE flag and found there is
> no other related place that actually uses it.
> 2. please also see the tx_timestamping.c file[1]. The author similarly
> only turns on SOF_TIMESTAMPING_SOFTWARE report flag without turning on
> any useful generation flag we are familiar with, like
> SOF_TIMESTAMPING_TX_SOFTWARE, SOF_TIMESTAMPING_TX_SCHED,
> SOF_TIMESTAMPING_TX_ACK.
> 
> [1]: https://github.com/Xilinx-CNS/onload/blob/master/src/tests/onload/hwtimestamping/tx_timestamping.c#L247
> 
> >
> > https://github.com/Xilinx-CNS/onload/blob/master/src/tests/onload/hwtimestamping/rx_timestamping.c
> >
> > I suspect that there will be more of such examples in practice. In
> > which case we should scuttle this. Please do a search online for
> > SOF_TIMESTAMPING_SOFTWARE to scan for this pattern.
> 
> I feel that only the buggy program or some program particularly takes
> advantage of the global netstamp_needed_key...

My point is that I just happen to stumble on one open source example
of this behavior.

That is a strong indication that other applications may make the same
implicit assumption. Both open source, and the probably many more non
public users.

Rule #1 is to not break users.

Given that we even have proof that we would break users, we cannot
make this change, sorry.

A safer alternative is to define a new timestamp option flag that
opt-in enables this filter-if-SOF_TIMESTAMPING_RX_SOFTWARE is not
set behavior.