netdev - Re: SOF_TIMESTAMPING_OPT_ID is unreliable when sendmsg fails

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]

Date: Thu, 8 Feb 2024 12:05:17 -0800
From: Andy Lutomirski <luto@...capital.net>
To: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
Cc: Willem de Bruijn <willemb@...gle.com>, "David S. Miller" <davem@...emloft.net>, 
	Network Development <netdev@...r.kernel.org>, Jakub Kicinski <kuba@...nel.org>
Subject: Re: SOF_TIMESTAMPING_OPT_ID is unreliable when sendmsg fails

On Thu, Feb 8, 2024 at 11:55 AM Vadim Fedorenko
<vadim.fedorenko@...ux.dev> wrote:
>
> On 08/02/2024 18:02, Andy Lutomirski wrote:
> > I’ve been using OPT_ID-style timestamping for years, but for some
> > reason this issue only bit me last week: if sendmsg() fails on a UDP
> > or ping socket, sk_tskey is poorly.  It may or may not get incremented
> > by the failed sendmsg().
> >
> Well, there are several error paths, for sure. For the sockets you
> mention the increment of tskey happens at __ip{,6}_append_data. There
> are 2 different types of failures which can happen after the increment.
> The first is MTU check fail, another one is memory allocation failures.
> I believe we can move increment to a later position, after MTU check in
> both functions to avoid first type of problem.

For reasons that I still haven't deciphered, I'm sporadically getting
EHOSTUNREACH after the increment.  I can't find anything in the code
that would cause that, and every time I try to instrument it, it stops
happening :(  I sendmsg to the same destination several times in rapid
succession, and at most one of them will get EHOSTUNREACH.

>
> > I can think of at least three ways to improve this:
> >
> > 1. Make it so that the sequence number is genuinely only incremented
> > on success. This may be tedious to implement and may be nearly
> > impossible if there are multiple concurrent sendmsg() calls on the
> > same socket.
>
> Multiple concurrent sendmsg() should bring a lot of problems on user-
> space side. With current implementation the application has to track the
> value of tskey to check incoming TX timestamp later. But with parallel
> sendmsg() the app cannot be sure which value is assigned to which call
> even in case of proper track value synchronization. That brings us to
> the other solutions if we consider having parallel threads working with
> same socket. Or we can simply pretend that it's impossible and then fix
> error path to decrement tskey value.
> >
> > 2. Allow the user program to specify an explicit ID.  cmsg values are
> > variable length, so for datagram sockets, extending the
> > SO_TIMESTAMPING cmsg with 64 bits of sequence number to be used for
> > the TX timestamp on that particular packet might be a nice solution.
> >
>
> This option can be really useful in case of really parallel work with
> sockets.

I personally like this one the best.  Some care would be needed to
allow programs to detect the new functionality.  Any preferred way to
handle it?