netdev - Re: [PATCH net-next v2 1/2] net_tstamp: add SCM_TS_OPT_ID to provide OPT

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: <66d7759ba35b0_ce32f294db@willemb.c.googlers.com.notmuch>
Date: Tue, 03 Sep 2024 16:46:19 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Vadim Fedorenko <vadim.fedorenko@...ux.dev>, 
 Willem de Bruijn <willemdebruijn.kernel@...il.com>, 
 Willem de Bruijn <willemb@...gle.com>
Cc: Jakub Kicinski <kuba@...nel.org>, 
 Paolo Abeni <pabeni@...hat.com>, 
 David Ahern <dsahern@...nel.org>, 
 netdev@...r.kernel.org, 
 Jason Xing <kerneljasonxing@...il.com>
Subject: Re: [PATCH net-next v2 1/2] net_tstamp: add SCM_TS_OPT_ID to provide
 OPT_ID in control message

Vadim Fedorenko wrote:
> On 02/09/2024 16:51, Willem de Bruijn wrote:
> > Jason Xing wrote:
> >> On Mon, Sep 2, 2024 at 9:09 PM Vadim Fedorenko <vadfed@...a.com> wrote:
> >>>
> >>> SOF_TIMESTAMPING_OPT_ID socket option flag gives a way to correlate TX
> >>> timestamps and packets sent via socket. Unfortunately, there is no way
> >>> to reliably predict socket timestamp ID value in case of error returned
> >>> by sendmsg. For UDP sockets it's impossible because of lockless
> >>> nature of UDP transmit, several threads may send packets in parallel. In
> >>> case of RAW sockets MSG_MORE option makes things complicated. More
> >>> details are in the conversation [1].
> >>> This patch adds new control message type to give user-space
> >>> software an opportunity to control the mapping between packets and
> >>> values by providing ID with each sendmsg. This works fine for UDP
> >>> sockets only, and explicit check is added to control message parser.
> >>>
> >>> [1] https://lore.kernel.org/netdev/CALCETrU0jB+kg0mhV6A8mrHfTE1D1pr1SD_B9Eaa9aDPfgHdtA@mail.gmail.com/
> >>>
> >>> Signed-off-by: Vadim Fedorenko <vadfed@...a.com>

> There is one more issue here. sk_tsflags is u32 as well as
> sockcm_cookie::tsflags. But sock_tx_timestamp receives __u16 tsflags,
> usually filled with sockc.tsflags. It works now because
> SOF_TIMESTAMPING_OPT_ID_TCP is not checked in these functions, but it's
> wrong in general. Should I fix it in this series or it's better to do in
> a seperate one?

Good find!

I overlooked that when expanding sk_tsflags when adding the 17th flag,
SOF_TIMESTAMPING_OPT_ID_TCP, and increasing sk_tsflags from 16 to 32b,
in commit b534dc46c8ae.

Passing sockc instead of sockc.ts_flags in all cases will address
this.

This does mean that up until now SOF_TIMESTAMPING_OPT_ID_TCP could not
be passed as a sock cookie. We did not notice that, because it is
benign: the option selects which tcp field to use as base for tskey,
and is only active on setsockopt.