Skip to content

Commit 18e3ffd

Browse files
mrpregregkh
authored andcommitted
bpf: Fix same-register dst/src OOB read and pointer leak in sock_ops
[ Upstream commit 10f86a2 ] 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: fd09af0 ("bpf: sock_ops ctx access may stomp registers in corner case") Fixes: 84f44df ("bpf: sock_ops sk access may stomp registers when dst_reg = src_reg") Reported-by: Quan Sun <2022090917019@std.uestc.edu.cn> Reported-by: Yinhao Hu <dddddd@hust.edu.cn> Reported-by: Kaiyan Mei <M202472210@hust.edu.cn> Reported-by: Dongliang Mu <dzm91@hust.edu.cn> Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com> Closes: https://lore.kernel.org/bpf/6fe1243e-149b-4d3b-99c7-fcc9e2f75787@std.uestc.edu.cn/T/#u Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev> Acked-by: Martin KaFai Lau <martin.lau@kernel.org> Link: https://patch.msgid.link/20260407022720.162151-2-jiayuan.chen@linux.dev Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
1 parent bf26ad9 commit 18e3ffd

1 file changed

Lines changed: 4 additions & 2 deletions

File tree

net/core/filter.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10581,10 +10581,11 @@ static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
1058110581
si->dst_reg, si->dst_reg, \
1058210582
offsetof(OBJ, OBJ_FIELD)); \
1058310583
if (si->dst_reg == si->src_reg) { \
10584-
*insn++ = BPF_JMP_A(1); \
10584+
*insn++ = BPF_JMP_A(2); \
1058510585
*insn++ = BPF_LDX_MEM(BPF_DW, reg, si->src_reg, \
1058610586
offsetof(struct bpf_sock_ops_kern, \
1058710587
temp)); \
10588+
*insn++ = BPF_MOV64_IMM(si->dst_reg, 0); \
1058810589
} \
1058910590
} while (0)
1059010591

@@ -10618,10 +10619,11 @@ static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
1061810619
si->dst_reg, si->src_reg, \
1061910620
offsetof(struct bpf_sock_ops_kern, sk));\
1062010621
if (si->dst_reg == si->src_reg) { \
10621-
*insn++ = BPF_JMP_A(1); \
10622+
*insn++ = BPF_JMP_A(2); \
1062210623
*insn++ = BPF_LDX_MEM(BPF_DW, reg, si->src_reg, \
1062310624
offsetof(struct bpf_sock_ops_kern, \
1062410625
temp)); \
10626+
*insn++ = BPF_MOV64_IMM(si->dst_reg, 0); \
1062510627
} \
1062610628
} while (0)
1062710629

0 commit comments

Comments
 (0)