Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Merge branch 'properly-load-values-from-insn_arays-with-non-zero-offsets'

Anton Protopopov says:

====================
Properly load values from insn_arays with non-zero offsets

The PTR_TO_INSN is always loaded via BPF_LDX_MEM instruction.
However, the verifier doesn't properly verify such loads when the
offset is not zero. Fix this and extend selftests with more scenarios.

v2 -> v3:
* Add a C-level selftest which triggers a load with nonzero offset (Alexei)
* Rephrase commit messages a bit

v2: https://lore.kernel.org/bpf/20260402184647.988132-1-a.s.protopopov@gmail.com/

v1: https://lore.kernel.org/bpf/20260401161529.681755-1-a.s.protopopov@gmail.com
====================

Link: https://patch.msgid.link/20260406160141.36943-1-a.s.protopopov@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>

+130 -34
+20
kernel/bpf/verifier.c
··· 212 212 static bool is_trusted_reg(const struct bpf_reg_state *reg); 213 213 static inline bool in_sleepable_context(struct bpf_verifier_env *env); 214 214 static const char *non_sleepable_context_description(struct bpf_verifier_env *env); 215 + static void scalar32_min_max_add(struct bpf_reg_state *dst_reg, struct bpf_reg_state *src_reg); 216 + static void scalar_min_max_add(struct bpf_reg_state *dst_reg, struct bpf_reg_state *src_reg); 215 217 216 218 static bool bpf_map_ptr_poisoned(const struct bpf_insn_aux_data *aux) 217 219 { ··· 7860 7858 return false; 7861 7859 } 7862 7860 7861 + static void add_scalar_to_reg(struct bpf_reg_state *dst_reg, s64 val) 7862 + { 7863 + struct bpf_reg_state fake_reg; 7864 + 7865 + if (!val) 7866 + return; 7867 + 7868 + fake_reg.type = SCALAR_VALUE; 7869 + __mark_reg_known(&fake_reg, val); 7870 + 7871 + scalar32_min_max_add(dst_reg, &fake_reg); 7872 + scalar_min_max_add(dst_reg, &fake_reg); 7873 + dst_reg->var_off = tnum_add(dst_reg->var_off, fake_reg.var_off); 7874 + 7875 + reg_bounds_sync(dst_reg); 7876 + } 7877 + 7863 7878 /* check whether memory at (regno + off) is accessible for t = (read | write) 7864 7879 * if t==write, value_regno is a register which value is stored into memory 7865 7880 * if t==read, value_regno is a register which will receive the value from memory ··· 7958 7939 return -EACCES; 7959 7940 } 7960 7941 copy_register_state(&regs[value_regno], reg); 7942 + add_scalar_to_reg(&regs[value_regno], off); 7961 7943 regs[value_regno].type = PTR_TO_INSN; 7962 7944 } else { 7963 7945 mark_reg_unknown(env, regs, value_regno);
+79 -34
tools/testing/selftests/bpf/prog_tests/bpf_gotox.c
··· 317 317 318 318 static int __check_ldimm64_gotox_prog_load(struct bpf_insn *insns, 319 319 __u32 insn_cnt, 320 - __u32 off1, __u32 off2) 320 + int off1, int off2, int off3) 321 321 { 322 322 const __u32 values[] = {5, 7, 9, 11, 13, 15}; 323 323 const __u32 max_entries = ARRAY_SIZE(values); ··· 349 349 /* r1 += off2 */ 350 350 insns[2].imm = off2; 351 351 352 + /* r1 = *(r1 + off3) */ 353 + insns[3].off = off3; 354 + 352 355 ret = prog_load(insns, insn_cnt); 353 356 close(map_fd); 354 357 return ret; 355 358 } 356 359 357 - static void reject_offsets(struct bpf_insn *insns, __u32 insn_cnt, __u32 off1, __u32 off2) 360 + static void 361 + allow_offsets(struct bpf_insn *insns, __u32 insn_cnt, int off1, int off2, int off3) 362 + { 363 + LIBBPF_OPTS(bpf_test_run_opts, topts); 364 + int prog_fd, err; 365 + char s[128] = ""; 366 + 367 + prog_fd = __check_ldimm64_gotox_prog_load(insns, insn_cnt, off1, off2, off3); 368 + snprintf(s, sizeof(s), "__check_ldimm64_gotox_prog_load(%d,%d,%d)", off1, off2, off3); 369 + if (!ASSERT_GE(prog_fd, 0, s)) 370 + return; 371 + 372 + err = bpf_prog_test_run_opts(prog_fd, &topts); 373 + if (!ASSERT_OK(err, "test_run_opts err")) { 374 + close(prog_fd); 375 + return; 376 + } 377 + 378 + if (!ASSERT_EQ(topts.retval, (off1 + off2 + off3) / 8, "test_run_opts retval")) { 379 + close(prog_fd); 380 + return; 381 + } 382 + 383 + close(prog_fd); 384 + } 385 + 386 + static void 387 + reject_offsets(struct bpf_insn *insns, __u32 insn_cnt, int off1, int off2, int off3) 358 388 { 359 389 int prog_fd; 360 390 361 - prog_fd = __check_ldimm64_gotox_prog_load(insns, insn_cnt, off1, off2); 391 + prog_fd = __check_ldimm64_gotox_prog_load(insns, insn_cnt, off1, off2, off3); 362 392 if (!ASSERT_EQ(prog_fd, -EACCES, "__check_ldimm64_gotox_prog_load")) 363 393 close(prog_fd); 364 394 } ··· 406 376 * The program rewrites the offsets in the instructions below: 407 377 * r1 = &map + offset1 408 378 * r1 += offset2 409 - * r1 = *r1 379 + * r1 = *(r1 + offset3) 410 380 * gotox r1 411 381 */ 412 382 BPF_LD_IMM64_RAW(BPF_REG_1, BPF_PSEUDO_MAP_VALUE, 0), ··· 433 403 BPF_MOV64_IMM(BPF_REG_0, 5), 434 404 BPF_EXIT_INSN(), 435 405 }; 436 - int prog_fd, err; 437 - __u32 off1, off2; 406 + int off1, off2, off3; 438 407 439 - /* allow all combinations off1 + off2 < 6 */ 440 - for (off1 = 0; off1 < 6; off1++) { 441 - for (off2 = 0; off1 + off2 < 6; off2++) { 442 - LIBBPF_OPTS(bpf_test_run_opts, topts); 408 + /* allow all combinations off1 + off2 + off3 < 6 */ 409 + for (off1 = 0; off1 < 6; off1++) 410 + for (off2 = 0; off1 + off2 < 6; off2++) 411 + for (off3 = 0; off1 + off2 + off3 < 6; off3++) 412 + allow_offsets(insns, ARRAY_SIZE(insns), 413 + off1 * 8, off2 * 8, off3 * 8); 443 414 444 - prog_fd = __check_ldimm64_gotox_prog_load(insns, ARRAY_SIZE(insns), 445 - off1 * 8, off2 * 8); 446 - if (!ASSERT_GE(prog_fd, 0, "__check_ldimm64_gotox_prog_load")) 447 - return; 415 + /* allow for some offsets to be negative */ 416 + allow_offsets(insns, ARRAY_SIZE(insns), 8 * 3, 0, -(8 * 3)); 417 + allow_offsets(insns, ARRAY_SIZE(insns), 8 * 3, -(8 * 3), 0); 418 + allow_offsets(insns, ARRAY_SIZE(insns), 0, 8 * 3, -(8 * 3)); 419 + allow_offsets(insns, ARRAY_SIZE(insns), 8 * 4, 0, -(8 * 2)); 420 + allow_offsets(insns, ARRAY_SIZE(insns), 8 * 4, -(8 * 2), 0); 421 + allow_offsets(insns, ARRAY_SIZE(insns), 0, 8 * 4, -(8 * 2)); 448 422 449 - err = bpf_prog_test_run_opts(prog_fd, &topts); 450 - if (!ASSERT_OK(err, "test_run_opts err")) { 451 - close(prog_fd); 452 - return; 453 - } 423 + /* disallow negative sums of offsets */ 424 + reject_offsets(insns, ARRAY_SIZE(insns), 8 * 3, 0, -(8 * 4)); 425 + reject_offsets(insns, ARRAY_SIZE(insns), 8 * 3, -(8 * 4), 0); 426 + reject_offsets(insns, ARRAY_SIZE(insns), 0, 8 * 3, -(8 * 4)); 454 427 455 - if (!ASSERT_EQ(topts.retval, off1 + off2, "test_run_opts retval")) { 456 - close(prog_fd); 457 - return; 458 - } 428 + /* disallow the off1 to be negative in any case */ 429 + reject_offsets(insns, ARRAY_SIZE(insns), -8 * 1, 0, 0); 430 + reject_offsets(insns, ARRAY_SIZE(insns), -8 * 1, 8 * 1, 0); 431 + reject_offsets(insns, ARRAY_SIZE(insns), -8 * 1, 8 * 1, 8 * 1); 459 432 460 - close(prog_fd); 461 - } 462 - } 433 + /* reject off1 + off2 + off3 >= 6 */ 434 + reject_offsets(insns, ARRAY_SIZE(insns), 8 * 3, 8 * 3, 8 * 0); 435 + reject_offsets(insns, ARRAY_SIZE(insns), 8 * 7, 8 * 0, 8 * 0); 436 + reject_offsets(insns, ARRAY_SIZE(insns), 8 * 0, 8 * 7, 8 * 0); 437 + reject_offsets(insns, ARRAY_SIZE(insns), 8 * 3, 8 * 0, 8 * 3); 438 + reject_offsets(insns, ARRAY_SIZE(insns), 8 * 0, 8 * 3, 8 * 3); 463 439 464 - /* reject off1 + off2 >= 6 */ 465 - reject_offsets(insns, ARRAY_SIZE(insns), 8 * 3, 8 * 3); 466 - reject_offsets(insns, ARRAY_SIZE(insns), 8 * 7, 8 * 0); 467 - reject_offsets(insns, ARRAY_SIZE(insns), 8 * 0, 8 * 7); 440 + /* reject (off1 + off2) % 8 != 0, off3 % 8 != 0 */ 441 + reject_offsets(insns, ARRAY_SIZE(insns), 3, 3, 0); 442 + reject_offsets(insns, ARRAY_SIZE(insns), 7, 0, 0); 443 + reject_offsets(insns, ARRAY_SIZE(insns), 0, 7, 0); 444 + reject_offsets(insns, ARRAY_SIZE(insns), 0, 0, 7); 445 + } 468 446 469 - /* reject (off1 + off2) % 8 != 0 */ 470 - reject_offsets(insns, ARRAY_SIZE(insns), 3, 3); 471 - reject_offsets(insns, ARRAY_SIZE(insns), 7, 0); 472 - reject_offsets(insns, ARRAY_SIZE(insns), 0, 7); 447 + static void check_ldimm64_off_gotox_llvm(struct bpf_gotox *skel) 448 + { 449 + __u64 in[] = {0, 1, 2, 3, 4}; 450 + __u64 out[] = {1, 1, 5, 1, 1}; 451 + int i; 452 + 453 + for (i = 0; i < ARRAY_SIZE(in); i++) 454 + check_simple(skel, skel->progs.load_with_nonzero_offset, in[i], out[i]); 473 455 } 474 456 475 457 void test_bpf_gotox(void) ··· 537 495 538 496 if (test__start_subtest("check-ldimm64-off-gotox")) 539 497 __subtest(skel, check_ldimm64_off_gotox); 498 + 499 + if (test__start_subtest("check-ldimm64-off-gotox-llvm")) 500 + __subtest(skel, check_ldimm64_off_gotox_llvm); 540 501 541 502 bpf_gotox__destroy(skel); 542 503 }
+31
tools/testing/selftests/bpf/progs/bpf_gotox.c
··· 421 421 return __nonstatic_global(in_user); 422 422 } 423 423 424 + SEC("syscall") 425 + int load_with_nonzero_offset(struct simple_ctx *ctx) 426 + { 427 + void *jj[] = { &&l1, &&l2, &&l3 }; 428 + 429 + /* 430 + * This makes LLVM to generate a load from the jj map with an offset: 431 + * r1 = 0x0 ll 432 + * r1 = *(u64 *)(r1 + 0x10) 433 + * gotox r1 434 + */ 435 + if (ctx->x == 2) 436 + goto *jj[ctx->x]; 437 + 438 + ret_user = 1; 439 + return 1; 440 + 441 + l1: 442 + /* never reached, but leave it here to outsmart LLVM */ 443 + ret_user = 0; 444 + return 0; 445 + l2: 446 + /* never reached, but leave it here to outsmart LLVM */ 447 + ret_user = 3; 448 + return 3; 449 + l3: 450 + ret_user = 5; 451 + return 5; 452 + } 453 + 424 454 #else /* __BPF_FEATURE_GOTOX */ 425 455 426 456 #define SKIP_TEST(TEST_NAME) \ ··· 472 442 SKIP_TEST(use_nonstatic_global1); 473 443 SKIP_TEST(use_nonstatic_global2); 474 444 SKIP_TEST(use_nonstatic_global_other_sec); 445 + SKIP_TEST(load_with_nonzero_offset); 475 446 476 447 #endif /* __BPF_FEATURE_GOTOX */ 477 448