Re: [PATCH bpf v2 1/2] bpf: Fix same-register dst/src OOB read and pointer leak in sock_ops

[sun jian]

On Mon, Apr 6, 2026 at 11:14 AM Jiayuan Chen <jiayuan.chen@xxxxxxxxx> wrote:
>
> When a BPF sock_ops program accesses ctx fields with dst_reg == src_reg,
> the SOCK_OPS_GET_SK() and SOCK_OPS_GET_FIELD() macros fail to zero the
> destination register in the !fullsock / !locked_tcp_sock path.
>
> Both macros borrow a temporary register to check is_fullsock /
> is_locked_tcp_sock when dst_reg == src_reg, because dst_reg holds the
> ctx pointer. When the check is false (e.g., TCP_NEW_SYN_RECV state with
> a request_sock), dst_reg should be zeroed but is not, leaving the stale
> ctx pointer:
>
>  - SOCK_OPS_GET_SK: dst_reg retains the ctx pointer, passes NULL checks
>    as PTR_TO_SOCKET_OR_NULL, and can be used as a bogus socket pointer,
>    leading to stack-out-of-bounds access in helpers like
>    bpf_skc_to_tcp6_sock().
>
>  - SOCK_OPS_GET_FIELD: dst_reg retains the ctx pointer which the
>    verifier believes is a SCALAR_VALUE, leaking a kernel pointer.
>
> Fix both macros by:
>  - Changing JMP_A(1) to JMP_A(2) in the fullsock path to skip the
>    added instruction.
>  - Adding BPF_MOV64_IMM(si->dst_reg, 0) after the temp register
>    restore in the !fullsock path, placed after the restore because
>    dst_reg == src_reg means we need src_reg intact to read ctx->temp.
>
> Fixes: 84f44df664e9 ("bpf: sock_ops sk access may stomp registers when dst_reg = src_reg")
> Reported-by: Quan Sun <2022090917019@xxxxxxxxxxxxxxxx>
> Reported-by: Yinhao Hu <dddddd@xxxxxxxxxxx>
> Reported-by: Kaiyan Mei <M202472210@xxxxxxxxxxx>
> Reported-by: Dongliang Mu <dzm91@xxxxxxxxxxx>
> Closes: https://lore.kernel.org/bpf/6fe1243e-149b-4d3b-99c7-fcc9e2f75787@xxxxxxxxxxxxxxxx/T/#u
> Suggested-by: Emil Tsalapatis <emil@xxxxxxxxxxxxxxx>
> Signed-off-by: Jiayuan Chen <jiayuan.chen@xxxxxxxxx>
> ---
> ---
>  net/core/filter.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 78b548158fb05..53ce06ed4a88e 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -10581,10 +10581,11 @@ static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
>                                       si->dst_reg, si->dst_reg,               \
>                                       offsetof(OBJ, OBJ_FIELD));              \
>                 if (si->dst_reg == si->src_reg) {                             \
> -                       *insn++ = BPF_JMP_A(1);                               \
> +                       *insn++ = BPF_JMP_A(2);                               \
>                         *insn++ = BPF_LDX_MEM(BPF_DW, reg, si->src_reg,       \
>                                       offsetof(struct bpf_sock_ops_kern,      \
>                                       temp));                                 \
> +                       *insn++ = BPF_MOV64_IMM(si->dst_reg, 0);              \
>                 }                                                             \
>         } while (0)
>
> @@ -10618,10 +10619,11 @@ static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
>                                       si->dst_reg, si->src_reg,               \
>                                       offsetof(struct bpf_sock_ops_kern, sk));\
>                 if (si->dst_reg == si->src_reg) {                             \
> -                       *insn++ = BPF_JMP_A(1);                               \
> +                       *insn++ = BPF_JMP_A(2);                               \
>                         *insn++ = BPF_LDX_MEM(BPF_DW, reg, si->src_reg,       \
>                                       offsetof(struct bpf_sock_ops_kern,      \
>                                       temp));                                 \
> +                       *insn++ = BPF_MOV64_IMM(si->dst_reg, 0);              \
>                 }                                                             \
>         } while (0)
>
> --
> 2.43.0
>
>
Reviewed-by Sun Jian <sun.jian.kdev@xxxxxxxxx>