netdev - Re: [PATCH net-next v2 00/12] net-timestamp: bpf extension to equip applications transparently

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: <CAL+tcoCwQpM3mMsB3Trw0XrHoLcHqSFxU1LSs0AxUyiZc1wNgw@mail.gmail.com>
Date: Sun, 13 Oct 2024 11:43:48 +0800
From: Jason Xing <kerneljasonxing@...il.com>
To: 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, ast@...nel.org, 
	daniel@...earbox.net, andrii@...nel.org, martin.lau@...ux.dev, 
	eddyz87@...il.com, song@...nel.org, yonghong.song@...ux.dev, 
	john.fastabend@...il.com, kpsingh@...nel.org, sdf@...ichev.me, 
	haoluo@...gle.com, jolsa@...nel.org, bpf@...r.kernel.org, 
	netdev@...r.kernel.org, Jason Xing <kernelxing@...cent.com>
Subject: Re: [PATCH net-next v2 00/12] net-timestamp: bpf extension to equip
 applications transparently

On Sun, Oct 13, 2024 at 11:28 AM Jason Xing <kerneljasonxing@...il.com> wrote:
>
> On Sun, Oct 13, 2024 at 1:48 AM Willem de Bruijn
> <willemdebruijn.kernel@...il.com> wrote:
> >
> > Jason Xing wrote:
> > > From: Jason Xing <kernelxing@...cent.com>
> > >
> > > A few weeks ago, I planned to extend SO_TIMESTMAMPING feature by using
> > > tracepoint to print information (say, tstamp) so that we can
> > > transparently equip applications with this feature and require no
> > > modification in user side.
> > >
> > > Later, we discussed at netconf and agreed that we can use bpf for better
> > > extension, which is mainly suggested by John Fastabend and Willem de
> > > Bruijn. Many thanks here! So I post this series to see if we have a
> > > better solution to extend. My feeling is BPF is a good place to provide
> > > a way to add timestamping by administrators, without having to rebuild
> > > applications.
> > >
> > > This approach mostly relies on existing SO_TIMESTAMPING feature, users
> > > only needs to pass certain flags through bpf_setsocktop() to a separate
> > > tsflags. For TX timestamps, they will be printed during generation
> > > phase. For RX timestamps, we will wait for the moment when recvmsg() is
> > > called.
> > >
> > > After this series, we could step by step implement more advanced
> > > functions/flags already in SO_TIMESTAMPING feature for bpf extension.
> > >
> > > In this series, I only support TCP protocol which is widely used in
> > > SO_TIMESTAMPING feature.
> > >
> > > ---
> > > V2
> > > Link: https://lore.kernel.org/all/20241008095109.99918-1-kerneljasonxing@gmail.com/
> > > 1. Introduce tsflag requestors so that we are able to extend more in the
> > > future. Besides, it enables TX flags for bpf extension feature separately
> > > without breaking users. It is suggested by Vadim Fedorenko.
> > > 2. introduce a static key to control the whole feature. (Willem)
> > > 3. Open the gate of bpf_setsockopt for the SO_TIMESTAMPING feature in
> > > some TX/RX cases, not all the cases.
> > >
> > > Note:
> > > The main concern we've discussion in V1 thread is how to deal with the
> > > applications using SO_TIMESTAMPING feature? In this series, I allow both
> > > cases to happen at the same time, which indicates that even one
> > > applications setting SO_TIMESTAMPING can still be traced through BPF
> > > program. Please see patch [04/12].
> >
> > This revision does not address the main concern.
> >
> > An administrator installed BPF program can affect results of a process
> > using SO_TIMESTAMPING in ways that break it.
>
> Sorry, I didn't get it. How the following code snippet would break users?
>
> void __skb_tstamp_tx(struct sk_buff *orig_skb,
>                      const struct sk_buff *ack_skb,
>                      struct skb_shared_hwtstamps *hwtstamps,
>                      struct sock *sk, int tstype)
> {
>         if (!sk)
>                 return;
>
>         if (static_branch_unlikely(&bpf_tstamp_control))
>                 bpf_skb_tstamp_tx_output(sk, orig_skb, tstype, hwtstamps);
>
>         skb_tstamp_tx_output(orig_skb, ack_skb, hwtstamps, sk,
> tstype);
> }
>
> You can see, the application shipped with SO_TIMESTAMPING still prints
> timestamps even when the application stays in the attached cgroup
> directory.

I tested this by running "./txtimestamp -4 -L 127.0.0.1 -l 1000 -c 5"
in the bpf attached directory and it can correctly print the
timestamp. So it would not break users.

And surprisingly I found the key is not that right (ERROR: key 1000,
expected 999). I will investigate and fix it.

Thanks,
Jason