mirror of
https://github.com/torvalds/linux.git
synced 2026-11-07 06:17:02 +01:00
Merge branch 'bpf-zero-extend-the-index-in-inlined-bpf_get_func_arg'
Woojin Ji says:
====================
bpf: Zero-extend the index in inlined bpf_get_func_arg()
v2, from the review of v1:
- Shift the index as a 32-bit value instead of inserting an explicit
zero-extension (Alexei). The inlined sequence is one instruction shorter
and the bounds check is untouched, so no program is accepted or rejected
differently.
- The selftest is now a separate patch (Jiri). Patch 2 keeps Jiri's
Acked-by from v1; patch 1 does not, because the kernel patch changed.
v1, the problem: bpf_get_func_arg() takes a u32 index, but the inlined
expansion compared only the low 32 bits of R2 with the argument count and
then shifted the full 64-bit register to compute the context address. R2 is
ARG_ANYTHING, so an index of 1ULL << 32 passed the bounds check as index
zero while the load was performed at ctx + (1ULL << 35).
Patch 2 depends on patch 1: without it the wide index faults in the inlined
sequence and the guest panics.
Validation, re-run with this revision (the 32-bit shift kernel):
- Disposable x86_64 QEMU guest running a BTF-enabled kernel built with
clang 22, with tracing and bpf_testmod. test_progs output, verbatim:
#150 get_func_args_fsession_test:OK
#151 get_func_args_test:OK
Summary: 2/0 PASSED, 0 SKIPPED, 0/0 FAILED
The pairs in that line are top-level tests/subtests, so both tests pass
with nothing skipped or failed; TEST_PROGS_EXIT=0 and the guest dmesg is
clean.
- The same two tests on unpatched ff47652a4b fault in the inlined
sequence: a page fault in bpf_prog_..._test2, with the load performed at
ctx + (1ULL << 35), which panics the guest.
- bpftool xlated dumps of both test programs show the index shifted as a
32-bit value (w2 <<= 3) at all 32 inlined helper sites on this kernel,
and the 64-bit shift (r2 <<= 3) at all 32 on unpatched ff47652a4b.
v1: https://lore.kernel.org/bpf/179221432098.1890560.17043109037413683447.get-func-arg-index@gmail.com/
---
====================
Link: https://patch.msgid.link/20261018-get-func-arg-v2-0-048848b3eddf@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
This commit is contained in:
commit
2c79356007
|
|
@ -2489,7 +2489,7 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env)
|
|||
cnt = 2;
|
||||
}
|
||||
insn_buf[cnt++] = BPF_JMP32_REG(BPF_JGE, BPF_REG_2, BPF_REG_0, 6);
|
||||
insn_buf[cnt++] = BPF_ALU64_IMM(BPF_LSH, BPF_REG_2, 3);
|
||||
insn_buf[cnt++] = BPF_ALU32_IMM(BPF_LSH, BPF_REG_2, 3);
|
||||
insn_buf[cnt++] = BPF_ALU64_REG(BPF_ADD, BPF_REG_2, BPF_REG_1);
|
||||
insn_buf[cnt++] = BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_2, 0);
|
||||
insn_buf[cnt++] = BPF_STX_MEM(BPF_DW, BPF_REG_3, BPF_REG_0, 0);
|
||||
|
|
|
|||
|
|
@ -6,6 +6,40 @@
|
|||
|
||||
char _license[] SEC("license") = "GPL";
|
||||
|
||||
/* Keep the upper index bits: the inlined expansion must truncate them. */
|
||||
static long (*bpf_get_func_arg_wide)(void *ctx, __u64 n, __u64 *value) =
|
||||
(void *)BPF_FUNC_get_func_arg;
|
||||
|
||||
/* Both traced functions are called with arguments 2 and 3. */
|
||||
static __always_inline int check_wide_index(void *ctx)
|
||||
{
|
||||
__u64 a = 0, z = 0xdeadbeefdeadbeefULL;
|
||||
__s64 err;
|
||||
int ok = 1;
|
||||
|
||||
/* The index is truncated to u32, so these select arguments 0 and 1. */
|
||||
err = bpf_get_func_arg_wide(ctx, 1ULL << 32, &a);
|
||||
ok &= err == 0 && ((int)a == 2);
|
||||
err = bpf_get_func_arg_wide(ctx, (1ULL << 32) | 1, &a);
|
||||
ok &= err == 0 && a == 3;
|
||||
err = bpf_get_func_arg_wide(ctx, 1ULL << 59, &a);
|
||||
ok &= err == 0 && ((int)a == 2);
|
||||
err = bpf_get_func_arg_wide(ctx, (1ULL << 59) | 1, &a);
|
||||
ok &= err == 0 && a == 3;
|
||||
|
||||
/* Out of range: -EINVAL and no store into the destination. */
|
||||
err = bpf_get_func_arg_wide(ctx, (1ULL << 32) | 2, &z);
|
||||
ok &= err == -EINVAL && z == 0xdeadbeefdeadbeefULL;
|
||||
err = bpf_get_func_arg_wide(ctx, (1ULL << 32) | 0xffffffffULL, &z);
|
||||
ok &= err == -EINVAL && z == 0xdeadbeefdeadbeefULL;
|
||||
err = bpf_get_func_arg_wide(ctx, (1ULL << 59) | 2, &z);
|
||||
ok &= err == -EINVAL && z == 0xdeadbeefdeadbeefULL;
|
||||
err = bpf_get_func_arg_wide(ctx, (1ULL << 59) | 0xffffffffULL, &z);
|
||||
ok &= err == -EINVAL && z == 0xdeadbeefdeadbeefULL;
|
||||
|
||||
return ok;
|
||||
}
|
||||
|
||||
__u64 test1_result = 0;
|
||||
SEC("fentry/bpf_fentry_test1")
|
||||
int BPF_PROG(test1)
|
||||
|
|
@ -63,6 +97,7 @@ int BPF_PROG(test2)
|
|||
/* return value */
|
||||
err = bpf_get_func_ret(ctx, &ret);
|
||||
test2_result &= err == 0 && ret == 5;
|
||||
test2_result &= check_wide_index(ctx);
|
||||
return 0;
|
||||
}
|
||||
|
||||
|
|
@ -162,6 +197,7 @@ int BPF_PROG(tp_test2)
|
|||
/* not valid argument */
|
||||
err = bpf_get_func_arg(ctx, 2, &z);
|
||||
test6_result &= err == -EINVAL;
|
||||
test6_result &= check_wide_index(ctx);
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user