On Tue, Feb 11, 2025 at 2:55 PM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote: > > On 2/8/25 2:32 AM, Jason Xing wrote: > > Considering the potential invalid access issues, calling > > bpf_sock_ops_setsockopt/getsockopt, bpf_sock_ops_cb_flags_set, > > and the bpf_sock_ops_load_hdr_opt in the new timestamping > > callbacks will return -EOPNOTSUPP error value. > > The "why" part is mostly missing. Why they are not safe to be used in the TX > timestamping callbacks? > > > > > It also prevents the UDP socket trying to access TCP fields in > > the bpf extension for SO_TIMESTAMPING for the same consideration. > Let's remove this UDP part to avoid confusion. UDP has very little to do with > disabling the helpers here. > > "BPF_CALL" in the subject is not clear either. "BPF_CALL" can mean many things, > such as calling BPF helpers, calling BPF kfuncs, or calling its own BPF > subprograms, etc. In this case, it is the calling BPF helpers. > > (Subject) > bpf: Disable unsafe helpers in TX timestamping callbacks > > (Why) > New TX timestamping sock_ops callbacks will be added in the subsequent patch. > Some of the existing BPF helpers will not be safe to be used in the TX > timestamping callbacks. > > The bpf_sock_ops_setsockopt, bpf_sock_ops_getsockopt, and > bpf_sock_ops_cb_flags_set require owning the sock lock. TX timestamping > callbacks will not own the lock. > > The bpf_sock_ops_load_hdr_opt needs the skb->data pointing to the TCP header. > This will not be true in the TX timestamping callbacks. > > (What and How) > At the beginning of these helpers, this patch checks the bpf_sock->op to ensure > these helpers are used by the existing sock_ops callbacks only. Many thanks here! I will use them in the commit message. Thanks, Jason
