Skip to content

Commit 659b9ba

Browse files
kkdwivediAlexei Starovoitov
authored andcommitted
bpf: Check size for BTF-based ctx access of pointer members
Robert Morris reported the following program type which passes the verifier in [0]: SEC("struct_ops/bpf_cubic_init") void BPF_PROG(bpf_cubic_init, struct sock *sk) { asm volatile("r2 = *(u16*)(r1 + 0)"); // verifier should demand u64 asm volatile("*(u32 *)(r2 +1504) = 0"); // 1280 in some configs } The second line may or may not work, but the first instruction shouldn't pass, as it's a narrow load into the context structure of the struct ops callback. The code falls back to btf_ctx_access to ensure correctness and obtaining the types of pointers. Ensure that the size of the access is correctly checked to be 8 bytes, otherwise the verifier thinks the narrow load obtained a trusted BTF pointer and will permit loads/stores as it sees fit. Perform the check on size after we've verified that the load is for a pointer field, as for scalar values narrow loads are fine. Access to structs passed as arguments to a BPF program are also treated as scalars, therefore no adjustment is needed in their case. Existing verifier selftests are broken by this change, but because they were incorrect. Verifier tests for d_path were performing narrow load into context to obtain path pointer, had this program actually run it would cause a crash. The same holds for verifier_btf_ctx_access tests. [0]: https://lore.kernel.org/bpf/51338.1732985814@localhost Fixes: 9e15db6 ("bpf: Implement accurate raw_tp context access via BTF") Reported-by: Robert Morris <rtm@mit.edu> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com> Link: https://lore.kernel.org/r/20241212092050.3204165-2-memxor@gmail.com Signed-off-by: Alexei Starovoitov <ast@kernel.org>
1 parent 04789af commit 659b9ba

File tree

3 files changed

+10
-4
lines changed

3 files changed

+10
-4
lines changed

kernel/bpf/btf.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6543,6 +6543,12 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
65436543
return false;
65446544
}
65456545

6546+
if (size != sizeof(u64)) {
6547+
bpf_log(log, "func '%s' size %d must be 8\n",
6548+
tname, size);
6549+
return false;
6550+
}
6551+
65466552
/* check for PTR_TO_RDONLY_BUF_OR_NULL or PTR_TO_RDWR_BUF_OR_NULL */
65476553
for (i = 0; i < prog->aux->ctx_arg_info_size; i++) {
65486554
const struct bpf_ctx_arg_aux *ctx_arg_info = &prog->aux->ctx_arg_info[i];

tools/testing/selftests/bpf/progs/verifier_btf_ctx_access.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ __success __retval(0)
1111
__naked void btf_ctx_access_accept(void)
1212
{
1313
asm volatile (" \
14-
r2 = *(u32*)(r1 + 8); /* load 2nd argument value (int pointer) */\
14+
r2 = *(u64 *)(r1 + 8); /* load 2nd argument value (int pointer) */\
1515
r0 = 0; \
1616
exit; \
1717
" ::: __clobber_all);
@@ -23,7 +23,7 @@ __success __retval(0)
2323
__naked void ctx_access_u32_pointer_accept(void)
2424
{
2525
asm volatile (" \
26-
r2 = *(u32*)(r1 + 0); /* load 1nd argument value (u32 pointer) */\
26+
r2 = *(u64 *)(r1 + 0); /* load 1nd argument value (u32 pointer) */\
2727
r0 = 0; \
2828
exit; \
2929
" ::: __clobber_all);

tools/testing/selftests/bpf/progs/verifier_d_path.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ __success __retval(0)
1111
__naked void d_path_accept(void)
1212
{
1313
asm volatile (" \
14-
r1 = *(u32*)(r1 + 0); \
14+
r1 = *(u64 *)(r1 + 0); \
1515
r2 = r10; \
1616
r2 += -8; \
1717
r6 = 0; \
@@ -31,7 +31,7 @@ __failure __msg("helper call is not allowed in probe")
3131
__naked void d_path_reject(void)
3232
{
3333
asm volatile (" \
34-
r1 = *(u32*)(r1 + 0); \
34+
r1 = *(u64 *)(r1 + 0); \
3535
r2 = r10; \
3636
r2 += -8; \
3737
r6 = 0; \

0 commit comments

Comments
 (0)