[PATCH v2 bpf-next 0/2] bpf: Fix OOB read and add tests for load-acquire/store-release

[PATCH v2 bpf-next 0/2] bpf: Fix OOB read and add tests for load-acquire/store-release

[Kohei Enju]

This patch series addresses an out-of-bounds read issue in 
check_atomic_load/store() reported by syzkaller when an invalid register 
number (MAX_BPF_REG or greater) is used.

The first patch fixes the actual bug by changing the order of validity 
checks, ensuring register validity is checked before atomic_ptr_type_ok() 
is called.
It also updates some tests that were assuming the previous order of checks.

The second patch adds new tests specifically for the invalid register 
number case to prevent regression in the future.

Changes:
  v2: 
    - Just swap atomic_ptr_type_ok() and check_load_mem()/check_store_reg()
    - Update some tests that were assuming the previous order of checks
    - Add new tests specifically for the invalid register number
  v1: https://lore.kernel.org/bpf/20250314195619.23772-2-enjuk@amazon.com/

Reported-by: syzbot+a5964227adc0f904549c@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=a5964227adc0f904549c

Kohei Enju (2):
  bpf: Fix out-of-bounds read in check_atomic_load/store()
  selftests/bpf: Add selftests for load-acquire/store-release when
    register number is invalid

 kernel/bpf/verifier.c                         | 16 +++++++++--
 .../bpf/progs/verifier_load_acquire.c         | 26 +++++++++++++++--
 .../bpf/progs/verifier_store_release.c        | 28 +++++++++++++++++--
 3 files changed, 63 insertions(+), 7 deletions(-)

-- 
2.49.0

[PATCH v2 bpf-next 1/2] bpf: Fix out-of-bounds read in check_atomic_load/store()

[Kohei Enju]

syzbot reported the following splat [0].

In check_atomic_load/store(), register validity is not checked before
atomic_ptr_type_ok(). This causes the out-of-bounds read in is_ctx_reg()
called from atomic_ptr_type_ok() when the register number is MAX_BPF_REG
or greater.

Let's call check_load_mem()/check_store_reg() before atomic_ptr_type_ok()
to avoid the OOB read.

However, some tests introduced by commit ff3afe5da998 ("selftests/bpf: Add
selftests for load-acquire and store-release instructions") assume
calling atomic_ptr_type_ok() before checking register validity.
Therefore the swapping of order unintentionally changes verifier messages
of these tests.

For example in the test load_acquire_from_pkt_pointer(), expected message
is 'BPF_ATOMIC loads from R2 pkt is not allowed' although actual messages
are different.

  validate_msgs:FAIL:754 expect_msg
  VERIFIER LOG:
  =============
  Global function load_acquire_from_pkt_pointer() doesn't return scalar. Only those are supported.
  0: R1=ctx() R10=fp0
  ; asm volatile ( @ verifier_load_acquire.c:140
  0: (61) r2 = *(u32 *)(r1 +0)          ; R1=ctx() R2_w=pkt(r=0)
  1: (d3) r0 = load_acquire((u8 *)(r2 +0))
  invalid access to packet, off=0 size=1, R2(id=0,off=0,r=0)
  R2 offset is outside of the packet
  processed 2 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0
  =============
  EXPECTED   SUBSTR: 'BPF_ATOMIC loads from R2 pkt is not allowed'
  #505/19  verifier_load_acquire/load-acquire from pkt pointer:FAIL

This is because instructions in the test don't pass check_load_mem() and
therefore don't enter the atomic_ptr_type_ok() path.
In this case, we have to modify instructions so that they pass the
check_load_mem() and trigger atomic_ptr_type_ok().
Similarly for store-release tests, we need to modify instructions so that
they pass check_store_reg().

Like load_acquire_from_pkt_pointer(), modify instructions in:
  load_acquire_from_sock_pointer()
  store_release_to_ctx_pointer()
  store_release_to_pkt_pointer()

Also in store_release_to_sock_pointer(), check_store_reg() returns error
early and atomic_ptr_type_ok() is not triggered, since write to sock
pointer is not possible in general.
We might be able to remove the test, but for now let's leave it and just
change the expected message.

[0]
 BUG: KASAN: slab-out-of-bounds in is_ctx_reg kernel/bpf/verifier.c:6185 [inline]
 BUG: KASAN: slab-out-of-bounds in atomic_ptr_type_ok+0x3d7/0x550 kernel/bpf/verifier.c:6223
 Read of size 4 at addr ffff888141b0d690 by task syz-executor143/5842

 CPU: 1 UID: 0 PID: 5842 Comm: syz-executor143 Not tainted 6.14.0-rc3-syzkaller-gf28214603dc6 #0
 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 02/12/2025
 Call Trace:
  <TASK>
  __dump_stack lib/dump_stack.c:94 [inline]
  dump_stack_lvl+0x241/0x360 lib/dump_stack.c:120
  print_address_description mm/kasan/report.c:408 [inline]
  print_report+0x16e/0x5b0 mm/kasan/report.c:521
  kasan_report+0x143/0x180 mm/kasan/report.c:634
  is_ctx_reg kernel/bpf/verifier.c:6185 [inline]
  atomic_ptr_type_ok+0x3d7/0x550 kernel/bpf/verifier.c:6223
  check_atomic_store kernel/bpf/verifier.c:7804 [inline]
  check_atomic kernel/bpf/verifier.c:7841 [inline]
  do_check+0x89dd/0xedd0 kernel/bpf/verifier.c:19334
  do_check_common+0x1678/0x2080 kernel/bpf/verifier.c:22600
  do_check_main kernel/bpf/verifier.c:22691 [inline]
  bpf_check+0x165c8/0x1cca0 kernel/bpf/verifier.c:23821
  bpf_prog_load+0x1664/0x20e0 kernel/bpf/syscall.c:2967
  __sys_bpf+0x4ea/0x820 kernel/bpf/syscall.c:5811
  __do_sys_bpf kernel/bpf/syscall.c:5918 [inline]
  __se_sys_bpf kernel/bpf/syscall.c:5916 [inline]
  __x64_sys_bpf+0x7c/0x90 kernel/bpf/syscall.c:5916
  do_syscall_x64 arch/x86/entry/common.c:52 [inline]
  do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
  entry_SYSCALL_64_after_hwframe+0x77/0x7f
 RIP: 0033:0x7fa3ac86bab9
 Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 c1 17 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48
 RSP: 002b:00007ffe50fff5f8 EFLAGS: 00000246 ORIG_RAX: 0000000000000141
 RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007fa3ac86bab9
 RDX: 0000000000000094 RSI: 00004000000009c0 RDI: 0000000000000005
 RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000006
 R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
 R13: 431bde82d7b634db R14: 0000000000000001 R15: 0000000000000001
  </TASK>

 Allocated by task 5842:
  kasan_save_stack mm/kasan/common.c:47 [inline]
  kasan_save_track+0x3f/0x80 mm/kasan/common.c:68
  poison_kmalloc_redzone mm/kasan/common.c:377 [inline]
  __kasan_kmalloc+0x98/0xb0 mm/kasan/common.c:394
  kasan_kmalloc include/linux/kasan.h:260 [inline]
  __kmalloc_cache_noprof+0x243/0x390 mm/slub.c:4325
  kmalloc_noprof include/linux/slab.h:901 [inline]
  kzalloc_noprof include/linux/slab.h:1037 [inline]
  do_check_common+0x1ec/0x2080 kernel/bpf/verifier.c:22499
  do_check_main kernel/bpf/verifier.c:22691 [inline]
  bpf_check+0x165c8/0x1cca0 kernel/bpf/verifier.c:23821
  bpf_prog_load+0x1664/0x20e0 kernel/bpf/syscall.c:2967
  __sys_bpf+0x4ea/0x820 kernel/bpf/syscall.c:5811
  __do_sys_bpf kernel/bpf/syscall.c:5918 [inline]
  __se_sys_bpf kernel/bpf/syscall.c:5916 [inline]
  __x64_sys_bpf+0x7c/0x90 kernel/bpf/syscall.c:5916
  do_syscall_x64 arch/x86/entry/common.c:52 [inline]
  do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
  entry_SYSCALL_64_after_hwframe+0x77/0x7f

 The buggy address belongs to the object at ffff888141b0d000
  which belongs to the cache kmalloc-2k of size 2048
 The buggy address is located 312 bytes to the right of
  allocated 1368-byte region [ffff888141b0d000, ffff888141b0d558)

 The buggy address belongs to the physical page:
 page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x141b08
 head: order:3 mapcount:0 entire_mapcount:0 nr_pages_mapped:0 pincount:0
 flags: 0x57ff00000000040(head|node=1|zone=2|lastcpupid=0x7ff)
 page_type: f5(slab)
 raw: 057ff00000000040 ffff88801b042000 dead000000000100 dead000000000122
 raw: 0000000000000000 0000000080080008 00000000f5000000 0000000000000000
 head: 057ff00000000040 ffff88801b042000 dead000000000100 dead000000000122
 head: 0000000000000000 0000000080080008 00000000f5000000 0000000000000000
 head: 057ff00000000003 ffffea000506c201 ffffffffffffffff 0000000000000000
 head: 0000000000000008 0000000000000000 00000000ffffffff 0000000000000000
 page dumped because: kasan: bad access detected
 page_owner tracks the page as allocated
 page last allocated via order 3, migratetype Unmovable, gfp_mask 0xd20c0(__GFP_IO|__GFP_FS|__GFP_NOWARN|__GFP_NORETRY|__GFP_COMP|__GFP_NOMEMALLOC), pid 1, tgid 1 (swapper/0), ts 8909973200, free_ts 0
  set_page_owner include/linux/page_owner.h:32 [inline]
  post_alloc_hook+0x1f4/0x240 mm/page_alloc.c:1585
  prep_new_page mm/page_alloc.c:1593 [inline]
  get_page_from_freelist+0x3a8c/0x3c20 mm/page_alloc.c:3538
  __alloc_frozen_pages_noprof+0x264/0x580 mm/page_alloc.c:4805
  alloc_pages_mpol+0x311/0x660 mm/mempolicy.c:2270
  alloc_slab_page mm/slub.c:2423 [inline]
  allocate_slab+0x8f/0x3a0 mm/slub.c:2587
  new_slab mm/slub.c:2640 [inline]
  ___slab_alloc+0xc27/0x14a0 mm/slub.c:3826
  __slab_alloc+0x58/0xa0 mm/slub.c:3916
  __slab_alloc_node mm/slub.c:3991 [inline]
  slab_alloc_node mm/slub.c:4152 [inline]
  __kmalloc_cache_noprof+0x27b/0x390 mm/slub.c:4320
  kmalloc_noprof include/linux/slab.h:901 [inline]
  kzalloc_noprof include/linux/slab.h:1037 [inline]
  virtio_pci_probe+0x54/0x340 drivers/virtio/virtio_pci_common.c:689
  local_pci_probe drivers/pci/pci-driver.c:324 [inline]
  pci_call_probe drivers/pci/pci-driver.c:392 [inline]
  __pci_device_probe drivers/pci/pci-driver.c:417 [inline]
  pci_device_probe+0x6c5/0xa10 drivers/pci/pci-driver.c:451
  really_probe+0x2b9/0xad0 drivers/base/dd.c:658
  __driver_probe_device+0x1a2/0x390 drivers/base/dd.c:800
  driver_probe_device+0x50/0x430 drivers/base/dd.c:830
  __driver_attach+0x45f/0x710 drivers/base/dd.c:1216
  bus_for_each_dev+0x239/0x2b0 drivers/base/bus.c:370
  bus_add_driver+0x346/0x670 drivers/base/bus.c:678
 page_owner free stack trace missing

 Memory state around the buggy address:
  ffff888141b0d580: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
  ffff888141b0d600: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
 >ffff888141b0d680: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
                          ^
  ffff888141b0d700: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
  ffff888141b0d780: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc

Reported-by: syzbot+a5964227adc0f904549c@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=a5964227adc0f904549c
Tested-by: syzbot+a5964227adc0f904549c@syzkaller.appspotmail.com
Fixes: e24bbad29a8d ("bpf: Introduce load-acquire and store-release instructions")
Fixes: ff3afe5da998 ("selftests/bpf: Add selftests for load-acquire and store-release instructions")
Signed-off-by: Kohei Enju <enjuk@amazon.com>
---
 kernel/bpf/verifier.c                            | 16 ++++++++++++++--
 .../selftests/bpf/progs/verifier_load_acquire.c  | 12 ++++++++++--
 .../selftests/bpf/progs/verifier_store_release.c | 14 +++++++++++---
 3 files changed, 35 insertions(+), 7 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 41fd93db8258..8ad7338e726b 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -7788,6 +7788,12 @@ static int check_atomic_rmw(struct bpf_verifier_env *env,
 static int check_atomic_load(struct bpf_verifier_env *env,
 			     struct bpf_insn *insn)
 {
+	int err;
+
+	err = check_load_mem(env, insn, true, false, false, "atomic_load");
+	if (err)
+		return err;
+
 	if (!atomic_ptr_type_ok(env, insn->src_reg, insn)) {
 		verbose(env, "BPF_ATOMIC loads from R%d %s is not allowed\n",
 			insn->src_reg,
@@ -7795,12 +7801,18 @@ static int check_atomic_load(struct bpf_verifier_env *env,
 		return -EACCES;
 	}
 
-	return check_load_mem(env, insn, true, false, false, "atomic_load");
+	return 0;
 }
 
 static int check_atomic_store(struct bpf_verifier_env *env,
 			      struct bpf_insn *insn)
 {
+	int err;
+
+	err = check_store_reg(env, insn, true);
+	if (err)
+		return err;
+
 	if (!atomic_ptr_type_ok(env, insn->dst_reg, insn)) {
 		verbose(env, "BPF_ATOMIC stores into R%d %s is not allowed\n",
 			insn->dst_reg,
@@ -7808,7 +7820,7 @@ static int check_atomic_store(struct bpf_verifier_env *env,
 		return -EACCES;
 	}
 
-	return check_store_reg(env, insn, true);
+	return 0;
 }
 
 static int check_atomic(struct bpf_verifier_env *env, struct bpf_insn *insn)
diff --git a/tools/testing/selftests/bpf/progs/verifier_load_acquire.c b/tools/testing/selftests/bpf/progs/verifier_load_acquire.c
index 608097453832..1babe9ad9b43 100644
--- a/tools/testing/selftests/bpf/progs/verifier_load_acquire.c
+++ b/tools/testing/selftests/bpf/progs/verifier_load_acquire.c
@@ -139,10 +139,16 @@ __naked void load_acquire_from_pkt_pointer(void)
 {
 	asm volatile (
 	"r2 = *(u32 *)(r1 + %[xdp_md_data]);"
+	"r3 = *(u32 *)(r1 + %[xdp_md_data_end]);"
+	"r1 = r2;"
+	"r1 += 8;"
+	"if r1 >= r3 goto l0_%=;"
 	".8byte %[load_acquire_insn];" // w0 = load_acquire((u8 *)(r2 + 0));
+"l0_%=:  r0 = 0;"
 	"exit;"
 	:
 	: __imm_const(xdp_md_data, offsetof(struct xdp_md, data)),
+	  __imm_const(xdp_md_data_end, offsetof(struct xdp_md, data_end)),
 	  __imm_insn(load_acquire_insn,
 		     BPF_ATOMIC_OP(BPF_B, BPF_LOAD_ACQ, BPF_REG_0, BPF_REG_2, 0))
 	: __clobber_all);
@@ -172,12 +178,14 @@ __naked void load_acquire_from_sock_pointer(void)
 {
 	asm volatile (
 	"r2 = *(u64 *)(r1 + %[sk_reuseport_md_sk]);"
-	".8byte %[load_acquire_insn];" // w0 = load_acquire((u8 *)(r2 + 0));
+	// w0 = load_acquire((u8 *)(r2 + offsetof(struct bpf_sock, family)));
+	".8byte %[load_acquire_insn];"
 	"exit;"
 	:
 	: __imm_const(sk_reuseport_md_sk, offsetof(struct sk_reuseport_md, sk)),
 	  __imm_insn(load_acquire_insn,
-		     BPF_ATOMIC_OP(BPF_B, BPF_LOAD_ACQ, BPF_REG_0, BPF_REG_2, 0))
+		     BPF_ATOMIC_OP(BPF_B, BPF_LOAD_ACQ, BPF_REG_0, BPF_REG_2,
+				   offsetof(struct bpf_sock, family)))
 	: __clobber_all);
 }
 
diff --git a/tools/testing/selftests/bpf/progs/verifier_store_release.c b/tools/testing/selftests/bpf/progs/verifier_store_release.c
index f1c64c08f25f..cd6f1e5f378b 100644
--- a/tools/testing/selftests/bpf/progs/verifier_store_release.c
+++ b/tools/testing/selftests/bpf/progs/verifier_store_release.c
@@ -140,11 +140,13 @@ __naked void store_release_to_ctx_pointer(void)
 {
 	asm volatile (
 	"w0 = 0;"
-	".8byte %[store_release_insn];" // store_release((u8 *)(r1 + 0), w0);
+	// store_release((u8 *)(r1 + offsetof(struct __sk_buff, cb[0])), w0);
+	".8byte %[store_release_insn];"
 	"exit;"
 	:
 	: __imm_insn(store_release_insn,
-		     BPF_ATOMIC_OP(BPF_B, BPF_STORE_REL, BPF_REG_1, BPF_REG_0, 0))
+		     BPF_ATOMIC_OP(BPF_B, BPF_STORE_REL, BPF_REG_1, BPF_REG_0,
+				   offsetof(struct __sk_buff, cb[0])))
 	: __clobber_all);
 }
 
@@ -156,10 +158,16 @@ __naked void store_release_to_pkt_pointer(void)
 	asm volatile (
 	"w0 = 0;"
 	"r2 = *(u32 *)(r1 + %[xdp_md_data]);"
+	"r3 = *(u32 *)(r1 + %[xdp_md_data_end]);"
+	"r1 = r2;"
+	"r1 += 8;"
+	"if r1 >= r3 goto l0_%=;"
 	".8byte %[store_release_insn];" // store_release((u8 *)(r2 + 0), w0);
+"l0_%=:  r0 = 0;"
 	"exit;"
 	:
 	: __imm_const(xdp_md_data, offsetof(struct xdp_md, data)),
+	  __imm_const(xdp_md_data_end, offsetof(struct xdp_md, data_end)),
 	  __imm_insn(store_release_insn,
 		     BPF_ATOMIC_OP(BPF_B, BPF_STORE_REL, BPF_REG_2, BPF_REG_0, 0))
 	: __clobber_all);
@@ -185,7 +193,7 @@ __naked void store_release_to_flow_keys_pointer(void)
 
 SEC("sk_reuseport")
 __description("store-release to sock pointer")
-__failure __msg("BPF_ATOMIC stores into R2 sock is not allowed")
+__failure __msg("R2 cannot write into sock")
 __naked void store_release_to_sock_pointer(void)
 {
 	asm volatile (
-- 
2.48.1

[PATCH v2 bpf-next 2/2] selftests/bpf: Add selftests for load-acquire/store-release when register number is invalid

[Kohei Enju]

syzbot reported out-of-bounds read in check_atomic_load/store() when
the register number is MAX_BPF_REG or greater in this context:
    https://syzkaller.appspot.com/bug?extid=a5964227adc0f904549c

To avoid the issue from now on, let's add tests where the register
number is invalid for load-acquire/store-release.

By the way, I chose R11 as an invalid register but there's no particular
insistence on this choice as long as the register number is invalid.

Signed-off-by: Kohei Enju <enjuk@amazon.com>
---
 .../selftests/bpf/progs/verifier_load_acquire.c    | 14 ++++++++++++++
 .../selftests/bpf/progs/verifier_store_release.c   | 14 ++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/verifier_load_acquire.c b/tools/testing/selftests/bpf/progs/verifier_load_acquire.c
index 1babe9ad9b43..e3912d2c6f95 100644
--- a/tools/testing/selftests/bpf/progs/verifier_load_acquire.c
+++ b/tools/testing/selftests/bpf/progs/verifier_load_acquire.c
@@ -189,6 +189,20 @@ __naked void load_acquire_from_sock_pointer(void)
 	: __clobber_all);
 }
 
+SEC("socket")
+__description("load-acquire with invalid register R11")
+__failure __failure_unpriv __msg("R11 is invalid")
+__naked void load_acquire_with_invalid_reg(void)
+{
+	asm volatile (
+	".8byte %[load_acquire_insn];" // r0 = load_acquire((u64 *)(r11 + 0));
+	"exit;"
+	:
+	: __imm_insn(load_acquire_insn,
+		     BPF_ATOMIC_OP(BPF_DW, BPF_LOAD_ACQ, BPF_REG_0, 11 /* invalid reg */, 0))
+	: __clobber_all);
+}
+
 #else /* CAN_USE_LOAD_ACQ_STORE_REL */
 
 SEC("socket")
diff --git a/tools/testing/selftests/bpf/progs/verifier_store_release.c b/tools/testing/selftests/bpf/progs/verifier_store_release.c
index cd6f1e5f378b..2dc1d713b4a6 100644
--- a/tools/testing/selftests/bpf/progs/verifier_store_release.c
+++ b/tools/testing/selftests/bpf/progs/verifier_store_release.c
@@ -257,6 +257,20 @@ __naked void store_release_leak_pointer_to_map(void)
 	: __clobber_all);
 }
 
+SEC("socket")
+__description("store-release with invalid register R11")
+__failure __failure_unpriv __msg("R11 is invalid")
+__naked void store_release_with_invalid_reg(void)
+{
+	asm volatile (
+	".8byte %[store_release_insn];" // store_release((u64 *)(r11 + 0), r1);
+	"exit;"
+	:
+	: __imm_insn(store_release_insn,
+		     BPF_ATOMIC_OP(BPF_DW, BPF_STORE_REL, 11 /* invalid reg */, BPF_REG_1, 0))
+	: __clobber_all);
+}
+
 #else
 
 SEC("socket")
-- 
2.48.1

Re: [PATCH v2 bpf-next 1/2] bpf: Fix out-of-bounds read in check_atomic_load/store()

[Eduard Zingerman]

On Fri, 2025-03-21 at 19:59 +0900, Kohei Enju wrote:
> syzbot reported the following splat [0].
> 
> In check_atomic_load/store(), register validity is not checked before
> atomic_ptr_type_ok(). This causes the out-of-bounds read in is_ctx_reg()
> called from atomic_ptr_type_ok() when the register number is MAX_BPF_REG
> or greater.
> 
> Let's call check_load_mem()/check_store_reg() before atomic_ptr_type_ok()
> to avoid the OOB read.

[...]

>  Memory state around the buggy address:
>   ffff888141b0d580: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>   ffff888141b0d600: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>  >ffff888141b0d680: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>                           ^
>   ffff888141b0d700: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>   ffff888141b0d780: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> 
> Reported-by: syzbot+a5964227adc0f904549c@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=a5964227adc0f904549c
> Tested-by: syzbot+a5964227adc0f904549c@syzkaller.appspotmail.com
> Fixes: e24bbad29a8d ("bpf: Introduce load-acquire and store-release instructions")
> Fixes: ff3afe5da998 ("selftests/bpf: Add selftests for load-acquire and store-release instructions")
> Signed-off-by: Kohei Enju <enjuk@amazon.com>
> ---

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

[...]

Re: [PATCH v2 bpf-next 2/2] selftests/bpf: Add selftests for load-acquire/store-release when register number is invalid

[Eduard Zingerman]

On Fri, 2025-03-21 at 19:59 +0900, Kohei Enju wrote:

Hi Kohei,

Thank you for adding these tests.

[...]

> +SEC("socket")
> +__description("load-acquire with invalid register R11")
> +__failure __failure_unpriv __msg("R11 is invalid")
> +__naked void load_acquire_with_invalid_reg(void)
> +{
> +	asm volatile (
> +	".8byte %[load_acquire_insn];" // r0 = load_acquire((u64 *)(r11 + 0));
> +	"exit;"
> +	:
> +	: __imm_insn(load_acquire_insn,
> +		     BPF_ATOMIC_OP(BPF_DW, BPF_LOAD_ACQ, BPF_REG_0, 11 /* invalid reg */, 0))
> +	: __clobber_all);
> +}
> +
>  #else /* CAN_USE_LOAD_ACQ_STORE_REL */
>  
>  SEC("socket")
> diff --git a/tools/testing/selftests/bpf/progs/verifier_store_release.c b/tools/testing/selftests/bpf/progs/verifier_store_release.c
> index cd6f1e5f378b..2dc1d713b4a6 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_store_release.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_store_release.c
> @@ -257,6 +257,20 @@ __naked void store_release_leak_pointer_to_map(void)
>  	: __clobber_all);
>  }
>  
> +SEC("socket")
> +__description("store-release with invalid register R11")
> +__failure __failure_unpriv __msg("R11 is invalid")
> +__naked void store_release_with_invalid_reg(void)
> +{
> +	asm volatile (
> +	".8byte %[store_release_insn];" // store_release((u64 *)(r11 + 0), r1);
> +	"exit;"
> +	:
> +	: __imm_insn(store_release_insn,
> +		     BPF_ATOMIC_OP(BPF_DW, BPF_STORE_REL, 11 /* invalid reg */, BPF_REG_1, 0))

On my machine / config, the value of 11 was too small to trigger the
KASAN warning. Value of 12 was sufficient.
Curious if it is my config, did you see KASAN warning locally when running this test
before applying the fix?
Maybe set the value to 15 here and above to maximize probability of KASAN warning?

> +	: __clobber_all);
> +}
> +
>  #else
>  
>  SEC("socket")

Re: [PATCH v2 bpf-next 2/2] selftests/bpf: Add selftests for load-acquire/store-release when register number is invalid

[Kohei Enju]

> [...]
> 
> > +SEC("socket")
> > +__description("load-acquire with invalid register R11")
> > +__failure __failure_unpriv __msg("R11 is invalid")
> > +__naked void load_acquire_with_invalid_reg(void)
> > +{
> > +	asm volatile (
> > +	".8byte %[load_acquire_insn];" // r0 = load_acquire((u64 *)(r11 + 0));
> > +	"exit;"
> > +	:
> > +	: __imm_insn(load_acquire_insn,
> > +		     BPF_ATOMIC_OP(BPF_DW, BPF_LOAD_ACQ, BPF_REG_0, 11 /* invalid reg */, 0))
> > +	: __clobber_all);
> > +}
> > +
> >  #else /* CAN_USE_LOAD_ACQ_STORE_REL */
> >  
> >  SEC("socket")
> > diff --git a/tools/testing/selftests/bpf/progs/verifier_store_release.c b/tools/testing/selftests/bpf/progs/verifier_store_release.c
> > index cd6f1e5f378b..2dc1d713b4a6 100644
> > --- a/tools/testing/selftests/bpf/progs/verifier_store_release.c
> > +++ b/tools/testing/selftests/bpf/progs/verifier_store_release.c
> > @@ -257,6 +257,20 @@ __naked void store_release_leak_pointer_to_map(void)
> >  	: __clobber_all);
> >  }
> >  
> > +SEC("socket")
> > +__description("store-release with invalid register R11")
> > +__failure __failure_unpriv __msg("R11 is invalid")
> > +__naked void store_release_with_invalid_reg(void)
> > +{
> > +	asm volatile (
> > +	".8byte %[store_release_insn];" // store_release((u64 *)(r11 + 0), r1);
> > +	"exit;"
> > +	:
> > +	: __imm_insn(store_release_insn,
> > +		     BPF_ATOMIC_OP(BPF_DW, BPF_STORE_REL, 11 /* invalid reg */, BPF_REG_1, 0))
> 
> On my machine / config, the value of 11 was too small to trigger the
> KASAN warning. Value of 12 was sufficient.
> Curious if it is my config, did you see KASAN warning locally when running this test
> before applying the fix?

Yes, as you pointed out, R11 doesn't trigger the KASAN splat in practice. 
For the splat, we need a value of 12 or larger.

The sizes of struct bpf_reg_state and bpf_func_state are 120 and 1368 
respectively.[1]
In the bpf_func_state, the member `regs` ranges from 0 to 1320 bytes (each 
120 bytes for each R0 to R10).
Also, the member `type`, which is accessed in is_ctx_reg(), is the first 
member of struct bpf_reg_state.

Therefore, when the register is R11, `regs->type` reads 4 bytes from 1320.
Since the size of bpf_func_state is 1368 and it doesn't exceed the end of 
the allocated memory, it doesn't trigger the KASAN splat.

OTOH, when the register is R12, `regs->type` reads 4 bytes from 1440 (120 
* 12 + 0).
This triggers the KASAN splat since it's larger than bpf_func_state's size.

Here is a part of the splat I saw in my environment when specifying R12. 
This says that the buggy address is 1440 (1368 + 72) and also matches 
previous analysis.

    The buggy address belongs to the object at ffff888112603800
     which belongs to the cache kmalloc-2k of size 2048
    The buggy address is located 72 bytes to the right of
     allocated 1368-byte region [ffff888112603800, ffff888112603d58)
    ...
    Memory state around the buggy address:
     ffff888112603c80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
     ffff888112603d00: 00 00 00 00 00 00 00 00 00 00 00 fc fc fc fc fc
    >ffff888112603d80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
                                   ^
     ffff888112603e00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
     ffff888112603e80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc

> Maybe set the value to 15 here and above to maximize probability of KASAN warning?

Understood. Thank you for the feedback.

I chose the minimum invalid register regardless of the actual occurrence 
of the splat, since the validity check of this type might be `regno >= 
MAX_BPF_REG` or not.
Sorry for my confusing choice.

Since I'm not attached to that particular choice, I'll change it to R15.
Thank you for reviewing and providing feedback!

> 
> > +	: __clobber_all);
> > +}
> > +
> >  #else
> >  
> >  SEC("socket")

Regards,
Kohei

---
[1]
struct bpf_reg_state {
        enum bpf_reg_type          type;                 /*     0     4 */
...

        /* size: 120, cachelines: 2, members: 19 */
        /* padding: 3 */
        /* last cacheline: 56 bytes */
};

struct bpf_func_state {
        struct bpf_reg_state       regs[11];             /*     0  1320 */
...
        int                        allocated_stack;      /*  1360     4 */

        /* size: 1368, cachelines: 22, members: 12 */
        /* sum members: 1363, holes: 1, sum holes: 1 */
        /* padding: 4 */
        /* last cacheline: 24 bytes */
};

Re: [PATCH v2 bpf-next 2/2] selftests/bpf: Add selftests for load-acquire/store-release when register number is invalid

[Eduard Zingerman]

On Sat, 2025-03-22 at 11:48 +0900, Kohei Enju wrote:

[...]

> I chose the minimum invalid register regardless of the actual occurrence 
> of the splat, since the validity check of this type might be `regno >= 
> MAX_BPF_REG` or not.
> Sorry for my confusing choice.
> 
> Since I'm not attached to that particular choice, I'll change it to R15.
> Thank you for reviewing and providing feedback!

Hi Kohei,

Thank you for detailed explanation.
Please add 'Acked-by: Eduard Zingerman <eddyz87@gmail.com>'
for the next revision.

Thanks,
Eduard

[...]