bpf: Prevent unsafe access to the sock fields in the BPF timestamping callback

The subsequent patch will implement BPF TX timestamping. It will
call the sockops BPF program without holding the sock lock.

This breaks the current assumption that all sock ops programs will
hold the sock lock. The sock's fields of the uapi's bpf_sock_ops
requires this assumption.

To address this, a new "u8 is_locked_tcp_sock;" field is added. This
patch sets it in the current sock_ops callbacks. The "is_fullsock"
test is then replaced by the "is_locked_tcp_sock" test during
sock_ops_convert_ctx_access().

The new TX timestamping callbacks added in the subsequent patch will
not have this set. This will prevent unsafe access from the new
timestamping callbacks.

Potentially, we could allow read-only access. However, this would
require identifying which callback is read-safe-only and also requires
additional BPF instruction rewrites in the covert_ctx. Since the BPF
program can always read everything from a socket (e.g., by using
bpf_core_cast), this patch keeps it simple and disables all read
and write access to any socket fields through the bpf_sock_ops
UAPI from the new TX timestamping callback.

Moreover, note that some of the fields in bpf_sock_ops are specific
to tcp_sock, and sock_ops currently only supports tcp_sock. In
the future, UDP timestamping will be added, which will also break
this assumption. The same idea used in this patch will be reused.
Considering that the current sock_ops only supports tcp_sock, the
variable is named is_locked_"tcp"_sock.

Signed-off-by: Jason Xing <kerneljasonxing@gmail.com>
Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
Link: https://patch.msgid.link/20250220072940.99994-4-kerneljasonxing@gmail.com

diff --git a/include/linux/filter.h b/include/linux/filter.h
index a3ea4628159572cbd9bacf6a19ade6cfb53efec3..d36d5d5180b1172818776c018babe82d8bd01574 100644 (file)
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -1508,6 +1508,7 @@ struct bpf_sock_ops_kern {
        void    *skb_data_end;
        u8      op;
        u8      is_fullsock;
+       u8      is_locked_tcp_sock;
        u8      remaining_opt_len;
        u64     temp;                   /* temp and everything after is not
                                         * initialized to 0 before calling

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 7fd2d7fa4532be29d0779354b9f555107c1790a5..94ea09faedf45c83bce5bdf751442066546a5622 100644 (file)
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -2657,6 +2657,7 @@ static inline int tcp_call_bpf(struct sock *sk, int op, u32 nargs, u32 *args)
        memset(&sock_ops, 0, offsetof(struct bpf_sock_ops_kern, temp));
        if (sk_fullsock(sk)) {
                sock_ops.is_fullsock = 1;
+               sock_ops.is_locked_tcp_sock = 1;
                sock_owned_by_me(sk);
        }
 

diff --git a/net/core/filter.c b/net/core/filter.c
index 5b7e44dfc037c6f3c38e928db6c20215ac1226ee..7a20ddae76676e766a3abecded801fc2ba745fd5 100644 (file)
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -10382,10 +10382,10 @@ static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
                }                                                             \
                *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(                       \
                                                struct bpf_sock_ops_kern,     \
-                                               is_fullsock),                 \
+                                               is_locked_tcp_sock),          \
                                      fullsock_reg, si->src_reg,              \
                                      offsetof(struct bpf_sock_ops_kern,      \
-                                              is_fullsock));                 \
+                                              is_locked_tcp_sock));          \
                *insn++ = BPF_JMP_IMM(BPF_JEQ, fullsock_reg, 0, jmp);         \
                if (si->dst_reg == si->src_reg)                               \
                        *insn++ = BPF_LDX_MEM(BPF_DW, reg, si->src_reg,       \
@@ -10470,10 +10470,10 @@ static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
                                               temp));                        \
                *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(                       \
                                                struct bpf_sock_ops_kern,     \
-                                               is_fullsock),                 \
+                                               is_locked_tcp_sock),          \
                                      reg, si->dst_reg,                       \
                                      offsetof(struct bpf_sock_ops_kern,      \
-                                              is_fullsock));                 \
+                                              is_locked_tcp_sock));          \
                *insn++ = BPF_JMP_IMM(BPF_JEQ, reg, 0, 2);                    \
                *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(                       \
                                                struct bpf_sock_ops_kern, sk),\

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 4686783b70defe0457571efd72d41ac88c528f7b..4a9e70e23ef8311507f12c03276d915d16f55020 100644 (file)
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -169,6 +169,7 @@ static void bpf_skops_parse_hdr(struct sock *sk, struct sk_buff *skb)
        memset(&sock_ops, 0, offsetof(struct bpf_sock_ops_kern, temp));
        sock_ops.op = BPF_SOCK_OPS_PARSE_HDR_OPT_CB;
        sock_ops.is_fullsock = 1;
+       sock_ops.is_locked_tcp_sock = 1;
        sock_ops.sk = sk;
        bpf_skops_init_skb(&sock_ops, skb, tcp_hdrlen(skb));
 
@@ -185,6 +186,7 @@ static void bpf_skops_established(struct sock *sk, int bpf_op,
        memset(&sock_ops, 0, offsetof(struct bpf_sock_ops_kern, temp));
        sock_ops.op = bpf_op;
        sock_ops.is_fullsock = 1;
+       sock_ops.is_locked_tcp_sock = 1;
        sock_ops.sk = sk;
        /* sk with TCP_REPAIR_ON does not have skb in tcp_finish_connect */
        if (skb)

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 464232a0d63796d5ac7437dafb5f496d5c05bc1c..a796cf451e55a0dc6b08f05facaefe3845043c5e 100644 (file)
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -525,6 +525,7 @@ static void bpf_skops_hdr_opt_len(struct sock *sk, struct sk_buff *skb,
                sock_owned_by_me(sk);
 
                sock_ops.is_fullsock = 1;
+               sock_ops.is_locked_tcp_sock = 1;
                sock_ops.sk = sk;
        }
 
@@ -570,6 +571,7 @@ static void bpf_skops_write_hdr_opt(struct sock *sk, struct sk_buff *skb,
                sock_owned_by_me(sk);
 
                sock_ops.is_fullsock = 1;
+               sock_ops.is_locked_tcp_sock = 1;
                sock_ops.sk = sk;
        }