netdev - Re: [PATCH net-next v4 10/11] net-timestamp: export the tskey for TCP bpf extension

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]

Message-ID: <CAL+tcoB7bxkXoUdZiVr2kXkZO0DHV3uZe2=JP2mHjGGy+T5WGQ@mail.gmail.com>
Date: Wed, 8 Jan 2025 20:55:35 +0800
From: Jason Xing <kerneljasonxing@...il.com>
To: Martin KaFai Lau <martin.lau@...ux.dev>
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org, 
	pabeni@...hat.com, dsahern@...nel.org, willemdebruijn.kernel@...il.com, 
	willemb@...gle.com, ast@...nel.org, daniel@...earbox.net, andrii@...nel.org, 
	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 v4 10/11] net-timestamp: export the tskey for TCP
 bpf extension

On Wed, Jan 8, 2025 at 12:21 PM Jason Xing <kerneljasonxing@...il.com> wrote:
>
> Hi Martin,
>
> > > -     bpf_skops_tx_timestamping(sk, skb, op, 2, args);
> > > +     if (sk_is_tcp(sk))
> > > +             args[2] = skb_shinfo(skb)->tskey;
> >
> > Instead of only passing one info "skb_shinfo(skb)->tskey" of a skb, pass the
> > whole skb ptr to the bpf prog. Take a look at bpf_skops_init_skb. Lets start
> > with end_offset = 0 for now so that the bpf prog won't use it to read the
> > skb->data. It can be revisited later.
> >
> >         bpf_skops_init_skb(&sock_ops, skb, 0);
> >
> > The bpf prog can use bpf_cast_to_kern_ctx() and bpf_core_cast() to get to the
> > skb_shinfo(skb). Take a look at the md_skb example in type_cast.c.
>
> In recent days, I've been working on this part. It turns out to be
> infeasible to pass "struct __sk_buff *skb" as the second parameter in
> skops_sockopt() in patch [11/11]. I cannot find a way to acquire the
> skb itself, sorry for that :(
>
> IIUC, there are three approaches to fetch the tskey:
> 1. Like what I wrote in this patchset, passing the tskey to bpf prog
> through calling __cgroup_bpf_run_filter_sock_ops() is simple and
> enough.
> 2. Considering future usability, I feel I can add skb_head, skb_end
> fields in sock_ops_convert_ctx_access().
> 3. Only adding a new field tskey like skb_hwtstamp in
> sock_ops_convert_ctx_access()

After considering those approaches over and over again, I think this
#3 is more suitable [1]. It can be aligned with other usages, say,
skb_hwtstamp. Normally, only temporary variables are passed through
calling __cgroup_bpf_run_filter_sock_ops(). Then we don't need to
worry about breakages on skb because of safety issues. Well, how about
this draft as below?

[1] diff patch
+       case offsetof(struct bpf_sock_ops, tskey): {
+               struct bpf_insn *jmp_on_null_skb;
+
+               *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(struct bpf_sock_ops_kern,
+                                                      skb),
+                                     si->dst_reg, si->src_reg,
+                                     offsetof(struct bpf_sock_ops_kern,
+                                              skb));
+               /* Reserve one insn to test skb == NULL */
+               jmp_on_null_skb = insn++;
+               insn = bpf_convert_shinfo_access(si->dst_reg,
si->dst_reg, insn);
+               *insn++ = BPF_LDX_MEM(BPF_DW, si->dst_reg, si->dst_reg,
+                                     bpf_target_off(struct skb_shared_info,
+                                                    tskey, 4,
+                                                    target_size));
+               *jmp_on_null_skb = BPF_JMP_IMM(BPF_JEQ, si->dst_reg, 0,
+                                              insn - jmp_on_null_skb - 1);
+               break;
+       }