[PATCH bpf 00/27] bpf: Fixes for the verifier, arena, skb and lwt helpers

[PATCH bpf 00/27] bpf: Fixes for the verifier, arena, skb and lwt helpers

[Alexei Starovoitov]

From: Alexei Starovoitov <ast@kernel.org>

These patches fix bugs in BPF:

  patches  area
  -------  ----------------------------------------------------------------
  01-02    stack traces: the limit of the trace length
  03-07    arena: the range of mmap() and of bpf_arena_free_pages()
  08-12    arguments of kfuncs and of extension progs
  13       cgroup detach: the permission check
  14       cgroup skb progs: the network offset
  15       bpf_skb_adjust_room(): the limit of gso_size
  16-24    lwt and seg6local End.BPF progs: pushed headers, skb->cb, dst
  25-27    skb offsets when a helper adds or removes room

10 of the patches are from Chris Mason and Josef Bacik. Patches 07 and 12
are selftests.

Signed-off-by: Alexei Starovoitov <ast@kernel.org>

Alexei Starovoitov (17):
  bpf: Clamp the trace length before the skip test in stackid_init()
  bpf: Refuse to map an arena at address 0
  bpf: Don't let bpf_arena_free_pages() see a half-set user range
  bpf: Refuse an arena mmap() longer than max_entries pages
  bpf: Remove the WARN from arena_get_unmapped_area()
  selftests/bpf: Test the length and the address of an arena mmap()
  bpf: Don't turn struct arguments of KF_ACQUIRE kfuncs into memory
  bpf: Don't turn kfunc arguments of an empty struct type into memory
  bpf: Compare the arguments of an extension with those of its target
  selftests/bpf: Test the arguments of an extension prog
  bpf: Don't let bpf_skb_adjust_room() raise gso_size to GSO_BY_FRAGS
  bpf: Set a dst for test runs of lwt_in progs
  bpf: Don't look for the SRH again in the End.BPF helpers
  bpf: Don't leave a stale MAC header in bpf_lwt_push_ip_encap()
  bpf: Reset skb->cb after an lwt prog pushes an IP header
  bpf: Require BPF_LWT_REROUTE when an lwt_in prog changes the protocol
  bpf: Don't let lwt_in progs call into IPv6 when it is disabled

Chris Mason (9):
  bpf: Reject memory for kfunc projection type args
  bpf: Require prog fd or capability for cgroup detach
  bpf, cgroup: Keep the skb network offset signed
  bpf: Reject non-IPv6 packets in BPF_LWT_ENCAP_SEG6
  bpf: Limit the SRH length in bpf_lwt_seg6_adjust_srh()
  bpf: Move the MAC header in bpf_lwt_seg6_adjust_srh()
  bpf: Move the transport header with the moved headers
  bpf: Use the inner transport header after a decap
  bpf: Check the offset in bpf_skb_generic_{push,pop}()

Josef Bacik (1):
  bpf: Clamp the trace length in __bpf_get_stack() again

 Documentation/bpf/kfuncs.rst                  |   7 +
 include/linux/bpf.h                           |  10 ++
 include/linux/bpf_verifier.h                  |   8 --
 include/linux/btf.h                           |   1 +
 kernel/bpf/arena.c                            |  16 ++-
 kernel/bpf/btf.c                              |  19 ++-
 kernel/bpf/cgroup.c                           |  26 +++-
 kernel/bpf/stackmap.c                         |  33 +++--
 kernel/bpf/verifier.c                         |  66 ++++++++-
 net/bpf/test_run.c                            |   3 +-
 net/core/filter.c                             | 133 ++++++++++++++----
 net/core/lwt_bpf.c                            |  53 ++++++-
 .../selftests/bpf/prog_tests/arena_mmap.c     |  37 +++++
 .../selftests/bpf/prog_tests/freplace_args.c  |  92 ++++++++++++
 .../selftests/bpf/progs/freplace_args.c       |  53 +++++++
 .../selftests/bpf/progs/freplace_args_tgt.c   |  70 +++++++++
 16 files changed, 564 insertions(+), 63 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/arena_mmap.c
 create mode 100644 tools/testing/selftests/bpf/prog_tests/freplace_args.c
 create mode 100644 tools/testing/selftests/bpf/progs/freplace_args.c
 create mode 100644 tools/testing/selftests/bpf/progs/freplace_args_tgt.c


base-commit: 5b96632457e4ccae88ecb751cf48176b2e3629a7
-- 
2.55.0

[PATCH bpf 01/27] bpf: Clamp the trace length in __bpf_get_stack() again

[Alexei Starovoitov]

From: Josef Bacik <josef@toxicpanda.com>

get_perf_callchain() and get_callchain_entry_for_task() store a trace in a
per-CPU entry, one per context. They release the entry before they return
it. Thus a second user in the same context can write to the entry while the
first user reads it. preempt_disable() stops only other tasks.

__bpf_get_stack() requests at most max_depth frames. This is skip plus the
frames that fit in the buffer, but not more than perf_event_max_stack.
Since the commit below, it copies 'trace->nr - skip' frames with no limit.
But a second user can store a longer trace. The unwinders of PowerPC add
context markers that max_depth does not limit. In both cases the copy
writes after the end of the buffer.

Read trace->nr one time. Clamp the value to max_depth before the skip test,
as __bpf_get_stack_pe() does. Thus when max_depth is less than skip, the
helper returns -EFAULT, and 'trace_nr - skip' does not underflow. The
copied frames can still be those of the second user.

Fixes: e17d62fedd10 ("bpf: Refactor stack map trace depth calculation into helper function")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 kernel/bpf/stackmap.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
index f7e8d766d128..7d90891cce0a 100644
--- a/kernel/bpf/stackmap.c
+++ b/kernel/bpf/stackmap.c
@@ -822,14 +822,20 @@ static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task,
 					   crosstask, false, 0);
 	}
 
-	if (unlikely(!trace) || trace->nr < skip) {
-		if (may_fault)
-			rcu_read_unlock();
-		preempt_enable();
-		goto err_fault;
-	}
+	if (unlikely(!trace))
+		goto err_unlock;
+
+	/*
+	 * trace->nr can be above max_depth. The per-CPU entry is released
+	 * already: a tracing program or an NMI which takes a callchain in the
+	 * same context before the copy below reuses it. And the unwinders of
+	 * PowerPC store context markers on top of max_depth.
+	 */
+	trace_nr = min_t(u32, READ_ONCE(trace->nr), max_depth);
+	if (trace_nr < skip)
+		goto err_unlock;
 
-	trace_nr = callchain_store(trace, trace->nr, buf, elem_size, flags);
+	trace_nr = callchain_store(trace, trace_nr, buf, elem_size, flags);
 
 	/* trace should not be dereferenced after this point */
 	if (may_fault)
@@ -838,6 +844,10 @@ static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task,
 
 	return callchain_finalize(buf, size, trace_nr, elem_size, flags, may_fault);
 
+err_unlock:
+	if (may_fault)
+		rcu_read_unlock();
+	preempt_enable();
 err_fault:
 	err = -EFAULT;
 clear:
-- 
2.55.0

[PATCH bpf 02/27] bpf: Clamp the trace length before the skip test in stackid_init()

[Alexei Starovoitov]

From: Alexei Starovoitov <ast@kernel.org>

stackid_init() gives a bucket of the stack map
min(trace_nr - skip, max_depth - skip) trace entries. Here max_depth is the
size of a bucket plus skip, but at most perf_event_max_stack. Context
markers can make trace_nr larger than that, so the skip test can pass when
max_depth < skip. Then 'max_depth - skip' wraps, and the kernel can write
after the bucket.

Clamp trace_nr to max_depth before the skip test, as __bpf_get_stack_pe()
does. The result changes only when skip >= max_depth. Then it is -EFAULT.
Until now skip == max_depth could give the id of an empty trace.

get_perf_callchain() releases the trace, so an NMI can change trace->nr.
Use READ_ONCE() in bpf_get_stackid(), so the compiler reads it one time.

Fixes: 23f852daa4ba ("bpf: Fix stackmap overflow check in __bpf_get_stackid()")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 kernel/bpf/stackmap.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
index 7d90891cce0a..a19126f5db02 100644
--- a/kernel/bpf/stackmap.c
+++ b/kernel/bpf/stackmap.c
@@ -522,12 +522,14 @@ static int stackid_init(struct stackid *stackid, struct bpf_map *map,
 	u32 skip = flags & BPF_F_SKIP_FIELD_MASK;
 	u32 max_depth;
 
+	max_depth = stack_map_calculate_max_depth(map->value_size, stack_map_data_size(map), flags);
+	/* trace_nr can be above max_depth and max_depth below skip: clamp first */
+	trace_nr = min_t(u32, trace_nr, max_depth);
 	if (trace_nr <= skip)
 		/* skipping more than usable stack trace */
 		return -EFAULT;
 
-	max_depth = stack_map_calculate_max_depth(map->value_size, stack_map_data_size(map), flags);
-	stackid->nr = min_t(u32, trace_nr - skip, max_depth - skip);
+	stackid->nr = trace_nr - skip;
 	stackid->len = stackid->nr * sizeof(u64);
 	stackid->ips = trace->ip + skip;
 	stackid->hash = jhash2((const u32 *)stackid->ips, stackid->len / sizeof(u32), 0);
@@ -642,7 +644,8 @@ BPF_CALL_3(bpf_get_stackid, struct pt_regs *, regs, struct bpf_map *, map,
 			/* couldn't fetch the stack trace */
 			return -EFAULT;
 
-		err = stackid_fastpath(&stackid, map, trace, trace->nr, flags);
+		/* the entry is released already, an NMI can change trace->nr */
+		err = stackid_fastpath(&stackid, map, trace, READ_ONCE(trace->nr), flags);
 		if (err != -ENOENT)
 			return err;
 
-- 
2.55.0

[PATCH bpf 03/27] bpf: Refuse to map an arena at address 0

[Alexei Starovoitov]

From: Alexei Starovoitov <ast@kernel.org>

The first mmap() of an arena sets user_vm_start and user_vm_end. Each
subsequent mmap() must use the same range. The value 0 in user_vm_start
means that the range is not set, but a mapping can start at address 0. Then
arena_map_mmap() does not check vm_start and sets user_vm_start again. The
range tree counts pages from user_vm_start. With a new user_vm_start it
reports allocated pages as free, and apply_range_set_cb() fails with
-EBUSY.

An arena at address 0 is not useful, because the verifier rejects all progs
that refer to it. Return -EINVAL from arena_get_unmapped_area(). It runs
before mmap() removes the old mappings in the range.

Fixes: 317460317a02 ("bpf: Introduce bpf_arena.")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 kernel/bpf/arena.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
index 3c61c181a6fa..ad8aab551093 100644
--- a/kernel/bpf/arena.c
+++ b/kernel/bpf/arena.c
@@ -650,6 +650,9 @@ static unsigned long arena_get_unmapped_area(struct file *filp, unsigned long ad
 	ret = mm_get_unmapped_area(filp, addr, len * 2, 0, flags);
 	if (IS_ERR_VALUE(ret))
 		return ret;
+	if (!ret)
+		/* user_vm_start == 0 means that the user range is not set yet */
+		return -EINVAL;
 	if ((ret >> 32) == ((ret + len - 1) >> 32))
 		return ret;
 	if (WARN_ON_ONCE(arena->user_vm_start))
-- 
2.55.0

Re: [PATCH bpf 03/27] bpf: Refuse to map an arena at address 0

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] bpf: round_up() overflow in arena_get_unmapped_area() maps arena at 0
--

commit 3145de230091dc6bb32f02294ed0d9f801a1c145
Author: Alexei Starovoitov <ast@kernel.org>

bpf: Refuse to map an arena at address 0

This commit rejects mapping a BPF arena at address 0 in
arena_get_unmapped_area() by returning -EINVAL when mm_get_unmapped_area()
returns 0, preventing arena->user_vm_start from remaining 0 after the
first mmap().

> diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
> index 3c61c181a6fa9..ad8aab5510939 100644
> --- a/kernel/bpf/arena.c
> +++ b/kernel/bpf/arena.c
> @@ -650,6 +650,9 @@ static unsigned long arena_get_unmapped_area(struct file *filp, unsigned long ad
>  	ret = mm_get_unmapped_area(filp, addr, len * 2, 0, flags);
>  	if (IS_ERR_VALUE(ret))
>  		return ret;
> +	if (!ret)
> +		/* user_vm_start == 0 means that the user range is not set yet */
> +		return -EINVAL;
>  	if ((ret >> 32) == ((ret + len - 1) >> 32))
>  		return ret;
>  	if (WARN_ON_ONCE(arena->user_vm_start))

[Severity: Medium]
Can arena_get_unmapped_area() still return 0 via integer overflow in the
subsequent round_up(ret, SZ_4G) when mmap() is called with MAP_FIXED?

In mm/mmap.c, __get_unmapped_area() invokes get_area() before checking
whether addr exceeds TASK_SIZE - len:

mm/mmap.c:__get_unmapped_area() {
	...
	if (get_area) {
		addr = get_area(file, addr, len, pgoff, flags);
	...
	if (IS_ERR_VALUE(addr))
		return addr;

	if (addr > TASK_SIZE - len)
		return -ENOMEM;
}

When arena->user_vm_start is 0 and userspace passes MAP_FIXED (or
MAP_FIXED_NOREPLACE) with a page-aligned addr in the range
[0xffffffff00001000UL, 0xfffffffffffff000UL], mm_get_unmapped_area()
returns addr directly without checking TASK_SIZE.

In kernel/bpf/arena.c:arena_get_unmapped_area():

	ret = mm_get_unmapped_area(filp, addr, len * 2, 0, flags);
	if (IS_ERR_VALUE(ret))
		return ret;
	if (!ret)
		/* user_vm_start == 0 means that the user range is not set yet */
		return -EINVAL;
	if ((ret >> 32) == ((ret + len - 1) >> 32))
		return ret;
	if (WARN_ON_ONCE(arena->user_vm_start))
		/* checks at map creation time should prevent this */
		return -EFAULT;
	return round_up(ret, SZ_4G);

Here IS_ERR_VALUE(ret) is false because ret is below -MAX_ERRNO, and !ret
is false. Because ret is a signed long, (ret >> 32) sign-extends to
0xffffffffffffffffUL while ((ret + len - 1) >> 32) is unsigned and
evaluates to 0xffffffffUL (or wraps to 0), so the 4 GiB check is also
false.

Execution then reaches round_up(ret, SZ_4G), which overflows 64-bit
arithmetic to 0. __get_unmapped_area() receives 0, passes the
addr > TASK_SIZE - len check, and maps the arena at address 0.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261101190535.1454146-1-alexei.starovoitov@gmail.com?part=3

[PATCH bpf 04/27] bpf: Don't let bpf_arena_free_pages() see a half-set user range

[Alexei Starovoitov]

From: Alexei Starovoitov <ast@kernel.org>

If map_extra is 0, the first arena_map_mmap() sets user_vm_start and
user_vm_end. It holds arena->lock, but bpf_arena_free_pages() reads both
fields without this mutex. The kfunc relies on the verifier, which rejects
a prog if user_vm_start of its arena is 0. But this check covers only the
maps in used_maps. The kfunc accepts any trusted pointer to struct bpf_map.

Thus the kfunc can run during arena_map_mmap() and read user_vm_start as 0
and user_vm_end as vma->vm_end. Then min() does not limit page_cnt to the
arena. arena_free_range() unmaps and frees pages of other vmalloc areas.

Read user_vm_start one time and return if it is 0. This value means that no
range is set, because arena_get_unmapped_area() rejects address 0. If only
user_vm_end is still 0, uaddr_end is 0 and the kfunc returns, as today.

Fixes: 317460317a02 ("bpf: Introduce bpf_arena.")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 kernel/bpf/arena.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
index ad8aab551093..e331130662be 100644
--- a/kernel/bpf/arena.c
+++ b/kernel/bpf/arena.c
@@ -991,6 +991,8 @@ static int arena_free_range(struct bpf_arena *arena, long uaddr, long page_cnt)
 
 static void arena_free_pages(struct bpf_arena *arena, long uaddr, long page_cnt, bool sleepable)
 {
+	/* The first mmap() may be setting the user range right now */
+	u64 user_vm_start = READ_ONCE(arena->user_vm_start);
 	struct mem_cgroup *new_memcg, *old_memcg;
 	u64 full_uaddr, uaddr_end;
 	struct arena_free_span *s;
@@ -998,8 +1000,8 @@ static void arena_free_pages(struct bpf_arena *arena, long uaddr, long page_cnt,
 	/* only aligned lower 32-bit are relevant */
 	uaddr = (u32)uaddr;
 	uaddr &= PAGE_MASK;
-	full_uaddr = clear_lo32(arena->user_vm_start) + uaddr;
-	if (full_uaddr < arena->user_vm_start)
+	full_uaddr = clear_lo32(user_vm_start) + uaddr;
+	if (!user_vm_start || full_uaddr < user_vm_start)
 		return;
 	uaddr_end = min(arena->user_vm_end, full_uaddr + (page_cnt << PAGE_SHIFT));
 	if (full_uaddr >= uaddr_end)
-- 
2.55.0

[PATCH bpf 05/27] bpf: Refuse an arena mmap() longer than max_entries pages

[Alexei Starovoitov]

From: Alexei Starovoitov <ast@kernel.org>

arena_map_alloc() puts max_entries free pages in the range tree of an
arena. But the arena kfuncs check their arguments against user_vm_start and
user_vm_end. Without map_extra, arena_map_mmap() takes both from the first
vma. This vma can be up to 4 GiB long. Then arena_free_pages() also marks
pages as free that were never in the range tree. Thus max_entries does not
limit the memory of the arena, but these pages are inside kern_vm.

Refuse an mmap() longer than max_entries pages with -E2BIG. With map_extra
this limit exists already. libbpf always maps max_entries pages. Replace
both SZ_4G checks with this limit, because it is at most 4 GiB.

Fixes: 317460317a02 ("bpf: Introduce bpf_arena.")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 kernel/bpf/arena.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
index e331130662be..c35b82f44f66 100644
--- a/kernel/bpf/arena.c
+++ b/kernel/bpf/arena.c
@@ -634,7 +634,7 @@ static unsigned long arena_get_unmapped_area(struct file *filp, unsigned long ad
 
 	if (pgoff)
 		return -EINVAL;
-	if (len > SZ_4G)
+	if (len > (u64)map->max_entries * PAGE_SIZE)
 		return -E2BIG;
 
 	/* if user_vm_start was specified at arena creation time */
@@ -682,7 +682,7 @@ static int arena_map_mmap(struct bpf_map *map, struct vm_area_struct *vma)
 		return -EBUSY;
 
 	/* Earlier checks should prevent this */
-	if (WARN_ON_ONCE(vma->vm_end - vma->vm_start > SZ_4G || vma->vm_pgoff))
+	if (WARN_ON_ONCE(vma_pages(vma) > map->max_entries || vma->vm_pgoff))
 		return -EFAULT;
 
 	if (remember_vma(arena, vma))
-- 
2.55.0

[PATCH bpf 06/27] bpf: Remove the WARN from arena_get_unmapped_area()

[Alexei Starovoitov]

From: Alexei Starovoitov <ast@kernel.org>

The user range of an arena must not cross a 4 GiB boundary. Thus
arena_get_unmapped_area() asks the mm for 2 * len bytes and rounds the
result up if necessary. If user_vm_start is not 0, addr must be equal to
it. A WARN_ON_ONCE() assumes that the mm returns addr. But addr is only a
hint without MAP_FIXED, and the mm can return a different range that
crosses a boundary. This is not a kernel bug.

Remove the WARN_ON_ONCE() and its -EFAULT. arena_map_mmap() already
compares vm_start with user_vm_start under arena->lock. If the rounded
result is user_vm_start, mmap() succeeds. If not, it returns -EBUSY.

Fixes: 317460317a02 ("bpf: Introduce bpf_arena.")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 kernel/bpf/arena.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
index c35b82f44f66..0b1ec3306ea4 100644
--- a/kernel/bpf/arena.c
+++ b/kernel/bpf/arena.c
@@ -655,9 +655,6 @@ static unsigned long arena_get_unmapped_area(struct file *filp, unsigned long ad
 		return -EINVAL;
 	if ((ret >> 32) == ((ret + len - 1) >> 32))
 		return ret;
-	if (WARN_ON_ONCE(arena->user_vm_start))
-		/* checks at map creation time should prevent this */
-		return -EFAULT;
 	return round_up(ret, SZ_4G);
 }
 
-- 
2.55.0

[PATCH bpf 07/27] selftests/bpf: Test the length and the address of an arena mmap()

[Alexei Starovoitov]

From: Alexei Starovoitov <ast@kernel.org>

libbpf maps all max_entries pages of an arena and never maps it at
address 0. No test maps an arena differently. Thus the checks from
"bpf: Refuse an arena mmap() longer than max_entries pages" and
"bpf: Refuse to map an arena at address 0" have no test. Test that a longer
mmap() fails with E2BIG, and that an mmap() at address 0 fails with EINVAL.

Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 .../selftests/bpf/prog_tests/arena_mmap.c     | 37 +++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/arena_mmap.c

diff --git a/tools/testing/selftests/bpf/prog_tests/arena_mmap.c b/tools/testing/selftests/bpf/prog_tests/arena_mmap.c
new file mode 100644
index 000000000000..2babc9ecf182
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/arena_mmap.c
@@ -0,0 +1,37 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <test_progs.h>
+#include <sys/mman.h>
+
+#define ARENA_PAGES 8
+
+static void test_refused(int pages, int flags, int err)
+{
+	LIBBPF_OPTS(bpf_map_create_opts, opts, .map_flags = BPF_F_MMAPABLE);
+	void *area;
+	int fd;
+
+	fd = bpf_map_create(BPF_MAP_TYPE_ARENA, "arena", 0, 0, ARENA_PAGES, &opts);
+	if (fd == -EOPNOTSUPP) {
+		test__skip();
+		return;
+	}
+	if (!ASSERT_GE(fd, 0, "bpf_map_create"))
+		return;
+
+	area = mmap(NULL, pages * getpagesize(), PROT_READ | PROT_WRITE,
+		    MAP_SHARED | flags, fd, 0);
+	if (ASSERT_EQ(area, MAP_FAILED, "mmap"))
+		ASSERT_EQ(errno, err, "errno");
+	else
+		munmap(area, pages * getpagesize());
+	close(fd);
+}
+
+void test_arena_mmap(void)
+{
+	if (test__start_subtest("long"))
+		test_refused(ARENA_PAGES + 1, 0, E2BIG);
+	if (test__start_subtest("address_0"))
+		test_refused(ARENA_PAGES, MAP_FIXED, EINVAL);
+}
-- 
2.55.0

Re: [PATCH bpf 07/27] selftests/bpf: Test the length and the address of an arena mmap()

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2321 bytes --]

> selftests/bpf: Test the length and the address of an arena mmap()

TL;DR: Test issue only: the new arena_mmap selftest may need to be
serial_test_arena_mmap(), like other arena tests, since parallel arena
creation under test_progs -jN can exhaust KASAN shadow memory on small CI VMs.

> diff --git a/tools/testing/selftests/bpf/prog_tests/arena_mmap.c b/tools/testing/selftests/bpf/prog_tests/arena_mmap.c
> new file mode 100644
> index 000000000000..2babc9ecf182
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/arena_mmap.c

[ ... ]

> +static void test_refused(int pages, int flags, int err)
> +{
> +	LIBBPF_OPTS(bpf_map_create_opts, opts, .map_flags = BPF_F_MMAPABLE);
> +	void *area;
> +	int fd;
> +
> +	fd = bpf_map_create(BPF_MAP_TYPE_ARENA, "arena", 0, 0, ARENA_PAGES, &opts);

[ ... ]

> +void test_arena_mmap(void)
> +{
> +	if (test__start_subtest("long"))
> +		test_refused(ARENA_PAGES + 1, 0, E2BIG);
> +	if (test__start_subtest("address_0"))
> +		test_refused(ARENA_PAGES, MAP_FIXED, EINVAL);
> +}

This isn't a regression in the mmap() checks themselves, but should this
entry point be serial_test_arena_mmap() instead of test_arena_mmap()?

Each subtest creates its own BPF_MAP_TYPE_ARENA map through
bpf_map_create(), and arena_map_alloc() reserves KERN_VM_SZ (SZ_4G +
GUARD_SZ) of vmalloc space for every arena with get_vm_area().

The other dedicated arena tests (arena_atomics, arena_direct_value,
arena_htab, arena_list, arena_spin_lock, arena_strsearch, arena_race,
arena_memcg, arena_mem_usage, libarena, libarena_asan) are all defined as
serial_test_*().  Commit a14f9ba080c1 ("selftests/bpf: Run arena tests
serially") made them serial because several arena tests running in parallel
add up their KASAN shadow memory and can exhaust a small CI VM during map
creation.

With the plain test_ prefix, test_arena_mmap() runs in the parallel phase of
test_progs -jN and adds two more arena creations alongside the other arena
tests.  Would renaming it to:

void serial_test_arena_mmap(void)

keep it consistent with the rest of the arena tests?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/41340841959

[PATCH bpf 08/27] bpf: Reject memory for kfunc projection type args

[Alexei Starovoitov]

From: Chris Mason <mason@kernel.org>

struct __sk_buff and struct xdp_md are projection types of struct sk_buff
and struct xdp_buff. A kfunc that takes a projection type casts the
argument to the underlying type. When the projection type is the ctx type
of the prog, the verifier accepts only the ctx. In other progs,
process_arg_ptr_to_btf_id() accepts a pointer to the underlying type.

But this function identifies the underlying type only by its name. A struct
of the same name in prog BTF also matches. Its objects are memory of the
prog. This test also ignores the offset of the register. Thus the kfunc
uses memory of the prog, or the middle of an skb, as an skb.

Accept the underlying type only from kernel BTF, and only at offset 0. A
prog that passes an skb of the kernel sees no change.

struct xdp_md has only scalar members. Thus in a prog with another ctx
type, resolve_func_arg_type() changes ARG_PTR_TO_BTF_ID to memory, and the
verifier accepts memory of the prog. Keep ARG_PTR_TO_BTF_ID for projection
types. The first Fixes tag is for this change. That commit merged the
conntrack kfunc sets of two prog types with different ctx types. Thus
bpf_xdp_ct_{alloc,lookup}() can use memory of the prog as an xdp_buff.

Fixes: a4703e318432 ("bpf: Switch to new kfunc flags infrastructure")
Fixes: ec209ad86324 ("bpf: verifier: Relax caller requirements for kfunc projection type args")
Signed-off-by: Chris Mason <mason@kernel.org>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 include/linux/btf.h   |  1 +
 kernel/bpf/btf.c      |  9 +++++++++
 kernel/bpf/verifier.c | 16 +++++++++++++---
 3 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/include/linux/btf.h b/include/linux/btf.h
index a18254b7521b..87fad4076f0e 100644
--- a/include/linux/btf.h
+++ b/include/linux/btf.h
@@ -631,6 +631,7 @@ s32 btf_find_dtor_kfunc(struct btf *btf, u32 btf_id);
 int register_btf_id_dtor_kfuncs(const struct btf_id_dtor_kfunc *dtors, u32 add_cnt,
 				struct module *owner);
 struct btf_struct_meta *btf_find_struct_meta(const struct btf *btf, u32 btf_id);
+bool btf_is_projection_type(const char *pname);
 bool btf_is_projection_of(const char *pname, const char *tname);
 bool btf_is_prog_ctx_type(struct bpf_verifier_log *log, const struct btf *btf,
 			   const struct btf_type *t, enum bpf_prog_type prog_type,
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 1be30487a2fa..fe24e459c1b1 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -6590,6 +6590,15 @@ static int find_kern_ctx_type_id(enum bpf_prog_type prog_type)
 	return ctx_type->type;
 }
 
+/*
+ * A projection type is the view that programs have of a kernel object with
+ * another layout: see btf_is_projection_of() for the two types behind them.
+ */
+bool btf_is_projection_type(const char *pname)
+{
+	return strcmp(pname, "__sk_buff") == 0 || strcmp(pname, "xdp_md") == 0;
+}
+
 bool btf_is_projection_of(const char *pname, const char *tname)
 {
 	if (strcmp(pname, "__sk_buff") == 0 && strcmp(tname, "sk_buff") == 0)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 26a5ee0708a3..53272f50226a 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -13585,7 +13585,13 @@ static int resolve_func_arg_type(struct bpf_verifier_env *env,
 	ref_t = btf_type_skip_modifiers(btf, ref_id, &ref_id);
 	ref_tname = btf_name_by_offset(btf, ref_t->name_off);
 
-	if (!btf_type_is_scalar_struct(env, btf, ref_t))
+	/*
+	 * A kfunc that takes a projection type (ie. xdp_md) casts it to the
+	 * underlying type. So memory of the program cannot stand in for it,
+	 * even if the projection type holds only scalars.
+	 */
+	if (btf_is_projection_type(ref_tname) ||
+	    !btf_type_is_scalar_struct(env, btf, ref_t))
 		return 0;
 
 	resolve_ret = btf_resolve_size(btf, ref_t, arg_size);
@@ -14340,9 +14346,13 @@ static int process_arg_ptr_to_btf_id(struct bpf_verifier_env *env, struct bpf_re
 
 	/* If kfunc is accepting a projection type (ie. __sk_buff), it cannot
 	 * actually use it -- it must cast to the underlying type. So we allow
-	 * caller to pass in the underlying type.
+	 * caller to pass in the underlying type. Only kernel BTF has that type:
+	 * a type of the same name in the BTF of the program is program memory.
+	 * And only a pointer to the start of the object points to that type.
 	 */
-	taking_projection = is_kfunc(meta) && btf_is_projection_of(arg_tname, reg_tname);
+	taking_projection = is_kfunc(meta) && btf_is_kernel(reg_btf) &&
+			    !reg->var_off.value &&
+			    btf_is_projection_of(arg_tname, reg_tname);
 	if (!taking_projection && !struct_same) {
 		verbose(env, "%s %s expected pointer to %s %s but %s has a pointer to %s %s\n",
 			meta->func_name, reg_arg_name(env, argno),
-- 
2.55.0

[PATCH bpf 09/27] bpf: Don't turn struct arguments of KF_ACQUIRE kfuncs into memory

[Alexei Starovoitov]

From: Alexei Starovoitov <ast@kernel.org>

When a kfunc takes a pointer to a struct that has only scalar members, the
verifier also accepts memory of the prog. Then resolve_func_arg_type()
changes ARG_PTR_TO_BTF_ID to ARG_PTR_TO_MEM. This is correct for a struct
of parameters. But struct bpf_cpumask also has only scalar members, and the
kernel allocates and frees it. Thus bpf_cpumask_acquire() returns memory of
the prog as a referenced object, and bpf_cpumask_release() can free it.

The verifier does not change the argument that a kfunc releases. KF_ACQUIRE
does not say which argument is the object. Thus do not change any argument
of a KF_ACQUIRE kfunc. Then the verifier rejects memory there. No other
KF_ACQUIRE kfunc takes memory in an ARG_PTR_TO_BTF_ID argument. Add to
kfuncs.rst that a KF_ACQUIRE kfunc must pair a struct of parameters with a
__sz argument.

The first commit below added the bug. Later the struct had pointers in an
rcu_head, so the verifier rejected memory. The second commit removed them.

Fixes: 516f4d3397c9 ("bpf: Enable cpumasks to be queried and used as kptrs")
Fixes: 8e07bb9ebcd9 ("bpf: Convert bpf_cpumask to bpf_mem_cache_free_rcu.")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 Documentation/bpf/kfuncs.rst | 7 +++++++
 kernel/bpf/verifier.c        | 5 +++++
 2 files changed, 12 insertions(+)

diff --git a/Documentation/bpf/kfuncs.rst b/Documentation/bpf/kfuncs.rst
index 73578a4c3b1f..329d1510b02a 100644
--- a/Documentation/bpf/kfuncs.rst
+++ b/Documentation/bpf/kfuncs.rst
@@ -407,6 +407,13 @@ referenced kptr (by invoking bpf_kptr_xchg). If not, the verifier fails the
 loading of the BPF program until no lingering references remain in all possible
 explored states of the program.
 
+A pointer to a struct in the arguments of a KF_ACQUIRE kfunc accepts only an
+object of that type. The same applies to the argument that a KF_RELEASE kfunc
+releases. For other arguments a kfunc also accepts memory of the program, if the
+struct has only scalar members. struct __sk_buff and struct xdp_md never accept
+memory. A KF_ACQUIRE kfunc which wants such memory has to pair the pointer with
+a __sz argument.
+
 2.5.2 KF_RET_NULL flag
 ----------------------
 
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 53272f50226a..cc2ba4dc9da3 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -13574,7 +13574,12 @@ static int resolve_func_arg_type(struct bpf_verifier_env *env,
 	if (base_type(*arg_type) != ARG_PTR_TO_BTF_ID)
 		return 0;
 
+	/*
+	 * A kfunc that releases an object, or that takes a reference to one,
+	 * needs the object itself: memory of the program cannot stand in for it.
+	 */
 	if (!is_kfunc(meta) || arg_type_is_release(*arg_type) ||
+	    is_kfunc_acquire(meta) ||
 	    base_type(reg->type) == PTR_TO_BTF_ID ||
 	    reg2btf_ids[base_type(reg->type)])
 		return 0;
-- 
2.55.0

[PATCH bpf 10/27] bpf: Don't turn kfunc arguments of an empty struct type into memory

[Alexei Starovoitov]

From: Alexei Starovoitov <ast@kernel.org>

A kfunc can take a pointer to a struct that has only scalar members. Then
the verifier also accepts memory of the prog: resolve_func_arg_type()
changes ARG_PTR_TO_BTF_ID to memory of the size of the struct. An empty
struct meets this condition too. But it is a handle for an object of
another type. The io_uring kfuncs cast the empty struct iou_ctx to struct
io_ring_ctx. Thus they use memory of the prog as a struct io_ring_ctx.

Keep ARG_PTR_TO_BTF_ID for a pointer to an empty struct. Then the verifier
accepts only a PTR_TO_BTF_ID of that struct, such as the ctx argument of
loop_step(). Only one other kfunc takes an empty struct, struct
bpf_ws_lock. It releases this argument, and the verifier does not change an
argument that the kfunc releases. Add the rule to kfuncs.rst.

Fixes: ec02fe217fa6 ("io_uring/bpf-ops: restrict ctx access to BPF")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 Documentation/bpf/kfuncs.rst | 6 +++---
 kernel/bpf/verifier.c        | 6 ++++--
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/Documentation/bpf/kfuncs.rst b/Documentation/bpf/kfuncs.rst
index 329d1510b02a..901c37504217 100644
--- a/Documentation/bpf/kfuncs.rst
+++ b/Documentation/bpf/kfuncs.rst
@@ -410,9 +410,9 @@ explored states of the program.
 A pointer to a struct in the arguments of a KF_ACQUIRE kfunc accepts only an
 object of that type. The same applies to the argument that a KF_RELEASE kfunc
 releases. For other arguments a kfunc also accepts memory of the program, if the
-struct has only scalar members. struct __sk_buff and struct xdp_md never accept
-memory. A KF_ACQUIRE kfunc which wants such memory has to pair the pointer with
-a __sz argument.
+struct has only scalar members. A struct without members, struct __sk_buff and
+struct xdp_md never accept memory. A KF_ACQUIRE kfunc which wants such memory
+has to pair the pointer with a __sz argument.
 
 2.5.2 KF_RET_NULL flag
 ----------------------
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index cc2ba4dc9da3..a456adaa459b 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -13593,10 +13593,12 @@ static int resolve_func_arg_type(struct bpf_verifier_env *env,
 	/*
 	 * A kfunc that takes a projection type (ie. xdp_md) casts it to the
 	 * underlying type. So memory of the program cannot stand in for it,
-	 * even if the projection type holds only scalars.
+	 * even if the projection type holds only scalars. A struct without
+	 * members (ie. iou_ctx) is a handle for an object of another type.
 	 */
 	if (btf_is_projection_type(ref_tname) ||
-	    !btf_type_is_scalar_struct(env, btf, ref_t))
+	    !btf_type_is_scalar_struct(env, btf, ref_t) ||
+	    !btf_type_vlen(ref_t))
 		return 0;
 
 	resolve_ret = btf_resolve_size(btf, ref_t, arg_size);
-- 
2.55.0

[PATCH bpf 11/27] bpf: Compare the arguments of an extension with those of its target

[Alexei Starovoitov]

From: Alexei Starovoitov <ast@kernel.org>

An extension replaces its target, a global function of another prog. From
BTF, btf_prepare_func_args() derives the kind of each argument, such as the
context, memory of a given size or a kernel object. The verifier checks the
callers with the BTF of the target and the extension with its own BTF.

Only btf_check_func_type_match() compares the two prototypes. For a pointer
to a struct, it compares only the struct names and assumes that the pointer
is the context. That was true until the commit below added pointers to
memory. Later the __arg_* tags added more kinds. Thus an extension can rely
on more than the callers guarantee (size, not NULL, kind). Then what the
verifier proved for it is false at run time.

Keep the result of btf_prepare_func_args() in bpf_func_info_aux and compare
the two results at load and on attach to another target. Return -EINVAL
unless the kind and the BTF id are the same and the memory is not larger.
The extension can also accept NULL where the callers never pass it. An
extension with the prototype and the tags of its target loads as before. An
argument that is the context only because of __arg_ctx needs the tag in the
extension too. The tag __arg_untrusted or __arg_arena on the extension only
is safe, but it is rejected because the kind differs.

Fixes: e5069b9c23b3 ("bpf: Support pointers in global func args")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 include/linux/bpf.h          | 10 +++++++++
 include/linux/bpf_verifier.h |  8 -------
 kernel/bpf/btf.c             | 10 ++++++---
 kernel/bpf/verifier.c        | 43 +++++++++++++++++++++++++++++++++++-
 4 files changed, 59 insertions(+), 12 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 1a8c3e514284..0b44bcb4f56e 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1752,12 +1752,22 @@ enum bpf_func_context {
 	BPF_FUNC_CTX_CNT,
 };
 
+struct bpf_subprog_arg_info {
+	enum bpf_arg_type arg_type;
+	union {
+		u32 mem_size;
+		u32 btf_id;
+	};
+};
+
 struct bpf_func_info_aux {
 	u16 linkage;
 	bool unreliable;
 	bool called[BPF_FUNC_CTX_CNT];
 	bool verified[BPF_FUNC_CTX_CNT];
 	bool called_with_pkt_ptrs;
+	/* what the callers of a global function are checked against */
+	struct bpf_subprog_arg_info args[MAX_BPF_FUNC_REG_ARGS];
 };
 
 enum bpf_jit_poke_reason {
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 7baeb831281f..3cb91c4787c9 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -923,14 +923,6 @@ struct bpf_func_ptr {
 /* the subprog that a bpf_func_ptr pointed to was removed as dead code */
 #define BPF_FUNC_PTR_DELETED ((u32)-1)
 
-struct bpf_subprog_arg_info {
-	enum bpf_arg_type arg_type;
-	union {
-		u32 mem_size;
-		u32 btf_id;
-	};
-};
-
 enum priv_stack_mode {
 	PRIV_STACK_UNKNOWN,
 	NO_PRIV_STACK,
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index fe24e459c1b1..8bf39098ba97 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -8492,9 +8492,8 @@ static int btf_check_func_type_match(struct bpf_verifier_log *log,
 		}
 		/* This is an optional check to make program writing easier.
 		 * Compare names of structs and report an error to the user.
-		 * btf_prepare_func_args() already checked that t2 struct
-		 * is a context type. btf_prepare_func_args() will check
-		 * later that t1 struct is a context type as well.
+		 * check_ext_args() compares what btf_prepare_func_args()
+		 * derives for the two arguments.
 		 */
 		s1 = btf_name_by_offset(btf1, t1->name_off);
 		s2 = btf_name_by_offset(btf2, t2->name_off);
@@ -9028,6 +9027,11 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
 		return err;
 	sub->arg_slot_cnt = slots_used;
 
+	/* check_ext_args() needs them after env is freed */
+	if (is_global)
+		memcpy(subprog_aux(env, subprog)->args, sub->args,
+		       sizeof(subprog_aux(env, subprog)->args));
+
 	sub->args_cached = true;
 
 	return 0;
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index a456adaa459b..1f19fbd627d2 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -22514,6 +22514,37 @@ static bool func_has_rcu_read_lock(const struct bpf_prog *prog, int subprog)
 	       !sub_aux->called[BPF_FUNC_CTX_NO_RCU_LOCK];
 }
 
+/*
+ * The callers of a global function are checked against the arguments that its
+ * BTF describes, and an extension is verified with those of its own BTF. It
+ * must not expect more than the callers of the function provide.
+ *
+ * While an extension is loaded, bpf_check_attach_target() runs before its
+ * arguments are known. check_attach_btf_id() does the check when they are.
+ */
+static int check_ext_args(struct bpf_verifier_log *log, const struct bpf_prog *prog,
+			  const struct bpf_prog *tgt_prog, int subprog)
+{
+	const struct bpf_subprog_arg_info *tgt = tgt_prog->aux->func_info_aux[subprog].args;
+	const struct bpf_subprog_arg_info *ext = prog->aux->func_info_aux[0].args;
+	int i;
+
+	for (i = 0; i < MAX_BPF_FUNC_REG_ARGS && ext[i].arg_type != ARG_UNUSED; i++) {
+		/* An extension may accept NULL where the callers never pass it. */
+		if ((ext[i].arg_type == tgt[i].arg_type ||
+		     ext[i].arg_type == (tgt[i].arg_type | PTR_MAYBE_NULL)) &&
+		    (base_type(ext[i].arg_type) == ARG_PTR_TO_MEM ?
+		     ext[i].mem_size <= tgt[i].mem_size : ext[i].btf_id == tgt[i].btf_id))
+			continue;
+
+		bpf_log(log, "Extension program's argument in R%d is incompatible with original\n",
+			i + 1);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 int bpf_check_attach_target(struct bpf_verifier_log *log,
 			    const struct bpf_prog *prog,
 			    const struct bpf_prog *tgt_prog,
@@ -22801,7 +22832,8 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
 			return -EINVAL;
 		}
 		if (prog_extension &&
-		    btf_check_type_match(log, prog, btf, t))
+		    (btf_check_type_match(log, prog, btf, t) ||
+		     check_ext_args(log, prog, tgt_prog, subprog)))
 			return -EINVAL;
 		t = btf_attach_func_proto(log, btf, btf_id);
 		if (!t || !btf_type_is_func_proto(t))
@@ -23021,6 +23053,15 @@ static int check_attach_btf_id(struct bpf_verifier_env *env)
 		 */
 		env->ops = bpf_verifier_ops[tgt_prog->type];
 		prog->expected_attach_type = tgt_prog->expected_attach_type;
+
+		/* only now it is known what the arguments of prog stand for */
+		ret = btf_prepare_func_args(env, 0);
+		if (ret)
+			return ret;
+		ret = check_ext_args(&env->log, prog, tgt_prog,
+				     find_tgt_subprog(tgt_prog, btf_id));
+		if (ret)
+			return ret;
 	}
 
 	/* store info about the attachment target that will be used later */
-- 
2.55.0

[PATCH bpf 12/27] selftests/bpf: Test the arguments of an extension prog

[Alexei Starovoitov]

From: Alexei Starovoitov <ast@kernel.org>

An extension must not expect more of its arguments than the callers of the
replaced function provide. It must fail if it expects more memory, a
non-NULL pointer or another kind of argument. The same argument, less
memory or a pointer that can be NULL must pass. Test check_ext_args() at
load and at attach to another function of the same target prog. The load
must fail with the message of the check, and the attach with -EINVAL.

Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 .../selftests/bpf/prog_tests/freplace_args.c  | 92 +++++++++++++++++++
 .../selftests/bpf/progs/freplace_args.c       | 53 +++++++++++
 .../selftests/bpf/progs/freplace_args_tgt.c   | 70 ++++++++++++++
 3 files changed, 215 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/freplace_args.c
 create mode 100644 tools/testing/selftests/bpf/progs/freplace_args.c
 create mode 100644 tools/testing/selftests/bpf/progs/freplace_args_tgt.c

diff --git a/tools/testing/selftests/bpf/prog_tests/freplace_args.c b/tools/testing/selftests/bpf/prog_tests/freplace_args.c
new file mode 100644
index 000000000000..d1930af23fa0
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/freplace_args.c
@@ -0,0 +1,92 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <test_progs.h>
+#include "freplace_args.skel.h"
+#include "freplace_args_tgt.skel.h"
+
+/*
+ * An extension must not expect more of its arguments than the callers of the
+ * function that it replaces provide. The progs are loaded and attached to each
+ * other. None of them runs.
+ */
+static const struct test {
+	const char *name;
+	const char *ext;	/* prog of freplace_args.c */
+	const char *tgt;	/* prog of freplace_args_tgt.c */
+	const char *func;	/* function of tgt that ext is loaded for */
+	const char *func2;	/* function of tgt that ext is attached to */
+	int bad_reg;		/* argument that is refused, 0: none */
+} tests[] = {
+	{ "same_nonnull",		"ext_mem_nonnull",  "tc_prog", "mem_nonnull" },
+	{ "same_trusted",		"ext_task_trusted", "tc_prog", "task_trusted" },
+	{ "smaller_struct",		"ext_mem_smaller",  "tc_prog", "mem_smaller" },
+	{ "larger_struct",		"ext_mem_larger",   "tc_prog", "mem_larger", NULL, 1 },
+	{ "nullable_for_nonnull",	"ext_mem",	    "tc_prog", "mem_nonnull" },
+	{ "nonnull_for_nullable",	"ext_mem_nonnull",  "tc_prog", "mem", NULL, 2 },
+	{ "trusted_for_mem",		"ext_task_trusted", "tc_prog", "task_mem", NULL, 1 },
+	{ "arg_ctx_for_ctx_type",	"ext_skb_ctx",	    "tc_prog", "skb_ctx" },
+	{ "mem_for_arg_ctx",		"ext_tp_mem",	    "tp_prog", "tagged_ctx", NULL, 1 },
+	{ "attach_nullable_to_nonnull",	"ext_mem",	    "tc_prog", "mem", "mem_nonnull" },
+	{ "attach_nonnull_to_nullable",	"ext_mem_nonnull",  "tc_prog", "mem_nonnull", "mem", 2 },
+};
+
+static void run_test(struct freplace_args_tgt *tgt_skel, const struct test *t)
+{
+	struct bpf_program *ext, *tgt;
+	struct freplace_args *skel;
+	char log[4096], msg[128];
+	struct bpf_link *link;
+	int err;
+
+	skel = freplace_args__open();
+	if (!ASSERT_OK_PTR(skel, "open"))
+		return;
+
+	ext = bpf_object__find_program_by_name(skel->obj, t->ext);
+	tgt = bpf_object__find_program_by_name(tgt_skel->obj, t->tgt);
+	if (!ASSERT_OK_PTR(ext, "ext") || !ASSERT_OK_PTR(tgt, "tgt"))
+		goto out;
+
+	log[0] = '\0';
+	bpf_program__set_autoload(ext, true);
+	/* libbpf prints the log of a load that fails and must not */
+	if (t->bad_reg && !t->func2)
+		bpf_program__set_log_buf(ext, log, sizeof(log));
+	err = bpf_program__set_attach_target(ext, bpf_program__fd(tgt), t->func);
+	if (!ASSERT_OK(err, "set_attach_target"))
+		goto out;
+
+	err = freplace_args__load(skel);
+	if (t->bad_reg && !t->func2) {
+		snprintf(msg, sizeof(msg),
+			 "Extension program's argument in R%d is incompatible with original",
+			 t->bad_reg);
+		if (ASSERT_ERR(err, "load"))
+			ASSERT_HAS_SUBSTR(log, msg, "log");
+		goto out;
+	}
+	if (!ASSERT_OK(err, "load") || !t->func2)
+		goto out;
+
+	link = bpf_program__attach_freplace(ext, bpf_program__fd(tgt), t->func2);
+	ASSERT_EQ(libbpf_get_error(link), t->bad_reg ? -EINVAL : 0, "attach");
+	bpf_link__destroy(link);
+out:
+	freplace_args__destroy(skel);
+}
+
+void test_freplace_args(void)
+{
+	struct freplace_args_tgt *tgt_skel;
+	int i;
+
+	tgt_skel = freplace_args_tgt__open_and_load();
+	if (!ASSERT_OK_PTR(tgt_skel, "tgt_open_and_load"))
+		return;
+
+	for (i = 0; i < ARRAY_SIZE(tests); i++) {
+		if (test__start_subtest(tests[i].name))
+			run_test(tgt_skel, &tests[i]);
+	}
+
+	freplace_args_tgt__destroy(tgt_skel);
+}
\ No newline at end of file
diff --git a/tools/testing/selftests/bpf/progs/freplace_args.c b/tools/testing/selftests/bpf/progs/freplace_args.c
new file mode 100644
index 000000000000..6e8f16bd5d8e
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/freplace_args.c
@@ -0,0 +1,53 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+
+/* In freplace_args_tgt.c each of them has 16 bytes. */
+struct buf { __u64 v[2]; };
+struct larger { __u64 v[4]; };
+struct smaller { __u64 v[1]; };
+struct tp_ctx { __u64 v[2]; };
+
+SEC("?freplace")
+int ext_mem(int a, struct buf *p)
+{
+	return 0;
+}
+
+SEC("?freplace")
+int ext_mem_nonnull(int a, struct buf *p __arg_nonnull)
+{
+	return 0;
+}
+
+SEC("?freplace")
+int ext_mem_larger(struct larger *p)
+{
+	return 0;
+}
+
+SEC("?freplace")
+int ext_mem_smaller(struct smaller *p)
+{
+	return 0;
+}
+
+SEC("?freplace")
+int ext_task_trusted(struct task_struct *t __arg_trusted)
+{
+	return 0;
+}
+
+SEC("?freplace")
+int ext_skb_ctx(struct __sk_buff *skb __arg_ctx)
+{
+	return 0;
+}
+
+SEC("?freplace")
+int ext_tp_mem(struct tp_ctx *p)
+{
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/progs/freplace_args_tgt.c b/tools/testing/selftests/bpf/progs/freplace_args_tgt.c
new file mode 100644
index 000000000000..4f1d1d3d2891
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/freplace_args_tgt.c
@@ -0,0 +1,70 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+
+/* In freplace_args.c 'larger' has 32 bytes and 'smaller' has 8 bytes. */
+struct buf { __u64 v[2]; };
+struct larger { __u64 v[2]; };
+struct smaller { __u64 v[2]; };
+struct tp_ctx { __u64 v[2]; };
+
+__weak int mem(int a, struct buf *p)
+{
+	return 0;
+}
+
+__weak int mem_nonnull(int a, struct buf *p __arg_nonnull)
+{
+	return 0;
+}
+
+__weak int mem_larger(struct larger *p)
+{
+	return 0;
+}
+
+__weak int mem_smaller(struct smaller *p)
+{
+	return 0;
+}
+
+/* No tag: memory of the size of the struct. */
+__weak int task_mem(struct task_struct *t)
+{
+	return 0;
+}
+
+__weak int task_trusted(struct task_struct *t __arg_trusted)
+{
+	return 0;
+}
+
+__weak int skb_ctx(struct __sk_buff *skb)
+{
+	return 0;
+}
+
+SEC("tc")
+int tc_prog(struct __sk_buff *skb)
+{
+	struct smaller smaller = {};
+	struct larger larger = {};
+	struct buf buf = {};
+
+	return mem(0, &buf) + mem_nonnull(0, &buf) + mem_larger(&larger) + mem_smaller(&smaller) +
+	       task_mem(NULL) + task_trusted(bpf_get_current_task_btf()) + skb_ctx(skb);
+}
+
+/* In a tracepoint prog the tag makes a context of any struct. */
+__weak int tagged_ctx(struct tp_ctx *ctx __arg_ctx)
+{
+	return 0;
+}
+
+SEC("tracepoint")
+int tp_prog(void *ctx)
+{
+	return tagged_ctx(ctx);
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.55.0

[PATCH bpf 13/27] bpf: Require prog fd or capability for cgroup detach

[Alexei Starovoitov]

From: Chris Mason <mason@kernel.org>

Since the commit below a cgroup prog detach does not require CAP_NET_ADMIN.
The fd of the attached prog gives the permission instead. But the detach
requires this fd only with BPF_F_ALLOW_MULTI. Without this flag a cgroup
has only one prog per attach type. Then find_detach_entry() returns this
prog and does not compare it with the prog of the caller. Thus the kernel
detaches this prog for a caller with neither the fd nor a capability.

Compare the two progs. If they are different, require CAP_NET_ADMIN or
CAP_SYS_ADMIN in the initial user namespace. The error is -EPERM. A detach
from tcx or netkit without a prog fd already requires these capabilities.
With BPF_F_ALLOW_MULTI nothing changes.

Without these capabilities, a caller of bpf_prog_detach() of libbpf must
now use bpf_prog_detach2() with the fd of the attached prog.

Fixes: 2c78ee898d8f ("bpf: Implement CAP_BPF")
Signed-off-by: Chris Mason <mason@kernel.org>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 kernel/bpf/cgroup.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index ce82963bcfc7..ca681d626c9e 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -1279,9 +1279,14 @@ static struct bpf_prog_list *find_detach_entry(struct hlist_head *progs,
 			return ERR_PTR(-ENOENT);
 
 		/* to maintain backward compatibility NONE and OVERRIDE cgroups
-		 * allow detaching with invalid FD (prog==NULL) in legacy mode
+		 * allow detaching with invalid FD (prog==NULL) in legacy mode.
+		 * Without the FD of the attached program the caller needs
+		 * CAP_NET_ADMIN or CAP_SYS_ADMIN in the initial user namespace.
 		 */
-		return hlist_entry(progs->first, typeof(*pl), node);
+		pl = hlist_entry(progs->first, typeof(*pl), node);
+		if (pl->prog != prog && !bpf_net_capable())
+			return ERR_PTR(-EPERM);
+		return pl;
 	}
 
 	if (!prog && !link)
-- 
2.55.0

Re: [PATCH bpf 13/27] bpf: Require prog fd or capability for cgroup detach

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 3031 bytes --]

> bpf: Require prog fd or capability for cgroup detach

TL;DR: The new -EPERM check on non-multi cgroup detach may be bypassable: a
caller without CAP_NET_ADMIN holding any prog fd could replace the attached
prog via BPF_PROG_ATTACH, then detach it. Should attach get the same check, or
the commit message say so?

> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> index ce82963bcfc7..ca681d626c9e 100644
> --- a/kernel/bpf/cgroup.c
> +++ b/kernel/bpf/cgroup.c
> @@ -1279,9 +1279,14 @@ static struct bpf_prog_list *find_detach_entry(struct hlist_head *progs,
>  			return ERR_PTR(-ENOENT);
>  
>  		/* to maintain backward compatibility NONE and OVERRIDE cgroups
> -		 * allow detaching with invalid FD (prog==NULL) in legacy mode
> +		 * allow detaching with invalid FD (prog==NULL) in legacy mode.
> +		 * Without the FD of the attached program the caller needs
> +		 * CAP_NET_ADMIN or CAP_SYS_ADMIN in the initial user namespace.
>  		 */
> -		return hlist_entry(progs->first, typeof(*pl), node);
> +		pl = hlist_entry(progs->first, typeof(*pl), node);
> +		if (pl->prog != prog && !bpf_net_capable())
> +			return ERR_PTR(-EPERM);

Is this check bypassable through BPF_PROG_ATTACH?

A caller without init-ns CAP_NET_ADMIN or CAP_SYS_ADMIN who holds a cgroup
fd and an fd for some other prog P2 of a matching type now gets -EPERM here,
but the non-multi attach path has no equivalent check:

kernel/bpf/cgroup.c:find_attach_entry() {
    ...
	if (!allow_multi) {
		if (hlist_empty(progs))
			return NULL;
		return hlist_entry(progs->first, typeof(*pl), node);
	}
    ...
}

bpf_prog_attach() -> cgroup_bpf_prog_attach() -> __cgroup_bpf_attach() only
checks hierarchy_allows_attach() and that the flags match the existing
ones.  It then overwrites pl->prog and calls bpf_prog_put(old_prog) on the
existing single entry.

Sequence:

root:    attach P1 to cgroup C with flags 0
caller:  BPF_PROG_DETACH(C, P2)         -> -EPERM with this patch
caller:  BPF_PROG_ATTACH(C, P2, 0)      -> replaces P1
caller:  BPF_PROG_DETACH(C, P2)         -> pl->prog == prog, succeeds

Such a caller can exist, for example when P2 was loaded through a BPF token
in a user namespace (bpf_prog_load() uses bpf_token_capable(token,
CAP_NET_ADMIN) for net admin prog types), or when a process loaded P2 and
then dropped its capabilities.

Multi-attach mode requires the fd of the prog being replaced (BPF_F_REPLACE
with replace_bpf_fd), but the non-multi path has no such requirement.  A
caller with no prog fd at all is still blocked, because it cannot attach.

This is pre-existing code that the patch does not touch, but should
non-multi replacement in find_attach_entry() get the same check?  Or should
the commit message say that holders of any prog fd are trusted to replace
the attached prog?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/41340841959

Re: [PATCH bpf 13/27] bpf: Require prog fd or capability for cgroup detach

[Alexei Starovoitov]

On Sun, Nov 01, 2026 at 11:38 AM bot+bpf-ci@kernel.org <bot+bpf-ci@kernel.org> wrote:
>> +		pl = hlist_entry(progs->first, typeof(*pl), node);
>> +		if (pl->prog != prog && !bpf_net_capable())
>> +			return ERR_PTR(-EPERM);
>
> Is this check bypassable through BPF_PROG_ATTACH?

No. That's not a bypass. The prog FD is the permission to attach.

> This is pre-existing code that the patch does not touch, but should
> non-multi replacement in find_attach_entry() get the same check?  Or should
> the commit message say that holders of any prog fd are trusted to replace
> the attached prog?

No. That check turns the documented replace into -EPERM.

[PATCH bpf 14/27] bpf, cgroup: Keep the skb network offset signed

[Alexei Starovoitov]

From: Chris Mason <mason@kernel.org>

cgroup_skb progs see a packet from its network header. While they run,
__cgroup_bpf_run_filter_skb() moves skb->data there. It assumes that the
network header is not after skb->data. It negates skb_network_offset(skb)
and pushes skb->data by this offset. The offset is an unsigned int.

But ip6mr_cache_report() and bpf_lwt_xmit_reroute() can pass an skb that
has data before the network header. Then the offset wraps. On a 64-bit
kernel skb->data is then far below skb->head.

skb->len has 32 bits and wraps to the correct value, thus only skb->data is
wrong. A read of packet data faults or gets other memory.

Keep the offset in an int. Pull skb->data when the network header is after
it. Otherwise push it, as today.

Fixes: 3007098494be ("cgroup: add support for eBPF programs")
Signed-off-by: Chris Mason <mason@kernel.org>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 kernel/bpf/cgroup.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index ca681d626c9e..57a785241d42 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -1790,7 +1790,7 @@ int __cgroup_bpf_run_filter_skb(struct sock *sk,
 				struct sk_buff *skb,
 				enum cgroup_bpf_attach_type atype)
 {
-	unsigned int offset = -skb_network_offset(skb);
+	int offset = skb_network_offset(skb);
 	struct sock *save_sk;
 	void *saved_data_end;
 	struct cgroup *cgrp;
@@ -1802,7 +1802,15 @@ int __cgroup_bpf_run_filter_skb(struct sock *sk,
 	cgrp = sock_cgroup_ptr(&sk->sk_cgrp_data);
 	save_sk = skb->sk;
 	skb->sk = sk;
-	__skb_push(skb, offset);
+
+	/*
+	 * The program sees the packet from its network header on, on
+	 * whichever side of skb->data that header lies.
+	 */
+	if (offset > 0)
+		__skb_pull(skb, offset);
+	else
+		__skb_push(skb, -offset);
 
 	/* compute pointers for the bpf prog */
 	bpf_compute_and_save_data_end(skb, &saved_data_end);
@@ -1844,7 +1852,10 @@ int __cgroup_bpf_run_filter_skb(struct sock *sk,
 			ret = -EFAULT;
 	}
 	bpf_restore_data_end(skb, saved_data_end);
-	__skb_pull(skb, offset);
+	if (offset > 0)
+		__skb_push(skb, offset);
+	else
+		__skb_pull(skb, -offset);
 	skb->sk = save_sk;
 
 	return ret;
-- 
2.55.0

[PATCH bpf 15/27] bpf: Don't let bpf_skb_adjust_room() raise gso_size to GSO_BY_FRAGS

[Alexei Starovoitov]

From: Alexei Starovoitov <ast@kernel.org>

When bpf_skb_adjust_room() shrinks a TCP GSO skb, it adds the number of
removed bytes to gso_size. gso_size has 16 bits, and nothing checks the
sum. Thus gso_size can wrap to 0 or to a small size, or it can be
GSO_BY_FRAGS. GSO_BY_FRAGS is not a size. It tells skb_segment() to read
the length of each segment from the frag_list. If the TCP GSO skb has no
frag_list, skb_segment() dereferences a NULL pointer.

Return -ENOTSUPP and do not change the skb if the sum is GSO_BY_FRAGS or
larger. With BPF_F_ADJ_ROOM_FIXED_GSO the helper does not change gso_size,
so it shrinks the skb as before.

Fixes: 2be7e212d541 ("bpf: add bpf_skb_adjust_room helper")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 net/core/filter.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/net/core/filter.c b/net/core/filter.c
index 303c0c44dc2c..c0fc9ff04bd5 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3762,6 +3762,14 @@ static int bpf_skb_net_shrink(struct sk_buff *skb, u32 off, u32 len_diff,
 			return -ENOTSUPP;
 	}
 
+	/*
+	 * The MSS is raised by len_diff. It has 16 bits, and GSO_BY_FRAGS is
+	 * not a size.
+	 */
+	if (skb_is_gso(skb) && !(flags & BPF_F_ADJ_ROOM_FIXED_GSO) &&
+	    skb_shinfo(skb)->gso_size + len_diff >= GSO_BY_FRAGS)
+		return -ENOTSUPP;
+
 	ret = skb_unclone(skb, GFP_ATOMIC);
 	if (unlikely(ret < 0))
 		return ret;
-- 
2.55.0

[PATCH bpf 16/27] bpf: Reject non-IPv6 packets in BPF_LWT_ENCAP_SEG6

[Alexei Starovoitov]

From: Chris Mason <mason@kernel.org>

The mode BPF_LWT_ENCAP_SEG6 of bpf_lwt_push_encap() pushes an IPv6 header
and an SRH, and it replaces the dst. The SRH says that IPv6 follows, and
skb->protocol does not change. Thus the mode assumes an IPv6 packet, but an
lwt_in prog also runs for IPv4 packets. After the prog, bpf_input() calls
orig_input() of the old dst. This IPv4 handler reads the pushed headers as
IPv4. It can write after the packet data, and the kernel crashes.

                 after the push         orig_input() expects
                 ---------------------  --------------------
  skb->protocol  ETH_P_IP               ETH_P_IP
  dst            rt6_info               rtable
  packet         | IPv6 | SRH | IPv4 |  | IPv4 |

The mode BPF_LWT_ENCAP_SEG6_INLINE returns -EBADMSG for non-IPv6 packets.
Apply this check to both modes. End.BPF does not change, because
seg6_local_input() drops non-IPv6 packets.

Fixes: fe94cc290f53 ("bpf: Add IPv6 Segment Routing helpers")
Signed-off-by: Chris Mason <mason@kernel.org>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 net/core/filter.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index c0fc9ff04bd5..a69f6817e273 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -6927,11 +6927,15 @@ static int bpf_push_seg6_encap(struct sk_buff *skb, u32 type, void *hdr, u32 len
 	if (!seg6_validate_srh(srh, len, false))
 		return -EINVAL;
 
+	/*
+	 * The encap mode names IPv6 as the inner protocol, and neither mode
+	 * updates skb->protocol.
+	 */
+	if (skb->protocol != htons(ETH_P_IPV6))
+		return -EBADMSG;
+
 	switch (type) {
 	case BPF_LWT_ENCAP_SEG6_INLINE:
-		if (skb->protocol != htons(ETH_P_IPV6))
-			return -EBADMSG;
-
 		err = seg6_do_srh_inline(skb, srh);
 		break;
 	case BPF_LWT_ENCAP_SEG6:
-- 
2.55.0

[PATCH bpf 17/27] bpf: Set a dst for test runs of lwt_in progs

[Alexei Starovoitov]

From: Alexei Starovoitov <ast@kernel.org>

When an lwt prog runs on a route, the skb always has a dst. To push an SRH,
bpf_lwt_push_encap() reads the device from this dst. But
bpf_prog_test_run_skb() sets a dst for lwt_xmit progs only. Thus the helper
dereferences a NULL dst in a test run of an lwt_in prog.

Set the same dst, ip6_null_entry, for lwt_in progs. When IPv6 is disabled
or not built, their test run now fails with -EOPNOTSUPP too. An lwt_out
prog cannot call this helper.

Fixes: fe94cc290f53 ("bpf: Add IPv6 Segment Routing helpers")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 net/bpf/test_run.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 3043a7393c29..d6c9dc62182e 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -1210,7 +1210,8 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
 		skb->ip_summed = CHECKSUM_COMPLETE;
 	}
 
-	if (prog->type == BPF_PROG_TYPE_LWT_XMIT) {
+	if (prog->type == BPF_PROG_TYPE_LWT_IN ||
+	    prog->type == BPF_PROG_TYPE_LWT_XMIT) {
 		if (!ipv6_mod_enabled()) {
 			pr_warn_once("Please test this program with IPv6 enabled kernel\n");
 			ret = -EOPNOTSUPP;
-- 
2.55.0

Re: [PATCH bpf 17/27] bpf: Set a dst for test runs of lwt_in progs

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2225 bytes --]

> bpf: Set a dst for test runs of lwt_in progs

TL;DR: Commit message only: the Fixes: tag may point at the wrong commit;
cd3092c7f8db, which first let lwt_in progs call the helper in test runs, looks
more accurate. Backport scope is the same either way.

> bpf: Set a dst for test runs of lwt_in progs
>
> When an lwt prog runs on a route, the skb always has a dst. To push an SRH,
> bpf_lwt_push_encap() reads the device from this dst. But
> bpf_prog_test_run_skb() sets a dst for lwt_xmit progs only. Thus the helper
> dereferences a NULL dst in a test run of an lwt_in prog.
>
> Set the same dst, ip6_null_entry, for lwt_in progs. When IPv6 is disabled
> or not built, their test run now fails with -EOPNOTSUPP too. An lwt_out
> prog cannot call this helper.
>
> Fixes: fe94cc290f53 ("bpf: Add IPv6 Segment Routing helpers")
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>

Does the Fixes: tag point at the commit that introduced this problem?

fe94cc290f53 added bpf_lwt_push_encap() and bpf_lwt_push_encap_proto, but no
get_func_proto callback returned that proto at that point.  The only
references to bpf_lwt_push_encap_proto in net/core/filter.c are its
definition, so no BPF program type could call the helper, and the NULL dst
was not reachable from a test run.

The immediate child, cd3092c7f8db ("bpf: Split lwt inout verifier
structures", same series, 2018-05-20), added lwt_in_func_proto() returning
&bpf_lwt_push_encap_proto and a separate lwt_in_prog_ops with
.test_run = bpf_prog_test_run_skb.  That is the commit that first let an
lwt_in prog call the helper under BPF_PROG_TEST_RUN, where
seg6_do_srh_encap() and seg6_do_srh_inline() dereference skb_dst(skb)->dev
with no dst set.

Should this be:

  Fixes: cd3092c7f8db ("bpf: Split lwt inout verifier structures")

The two commits are consecutive and ship in the same release, so the stable
backport scope is the same either way.  fe94cc290f53 is defensible as the
commit that added the dereferencing code.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/41340841959

Re: [PATCH bpf 17/27] bpf: Set a dst for test runs of lwt_in progs

[Alexei Starovoitov]

On Sun, Nov 01, 2026 at 11:38 AM bot+bpf-ci@kernel.org <bot+bpf-ci@kernel.org> wrote:
> TL;DR: Commit message only: the Fixes: tag may point at the wrong commit;
> cd3092c7f8db, which first let lwt_in progs call the helper in test runs, looks
> more accurate. Backport scope is the same either way.

no. the tag is correct.

[PATCH bpf 18/27] bpf: Limit the SRH length in bpf_lwt_seg6_adjust_srh()

[Alexei Starovoitov]

From: Chris Mason <mason@kernel.org>

bpf_lwt_seg6_adjust_srh() lets an End.BPF prog change the size of the TLVs
in the SRH. The helper keeps the SRH length in srh_state->hdrlen, in bytes
and without the first 8 bytes. Later seg6_bpf_has_valid_srh() writes it to
srh->hdrlen, an 8-bit field in units of 8 bytes. But srh_state->hdrlen has
no upper limit, and the cast to u8 can truncate it. After that,
seg6_validate_srh() gets the SRH length from srh->hdrlen, and thus it can
accept the SRH. Then srh->hdrlen is too small, and a parser reads TLVs as
the subsequent header.

Return -EFAULT when the new srh_state->hdrlen is larger than 2040, the
largest length that srh->hdrlen can hold. To replace a TLV in the largest
SRH, a prog can no longer add bytes first. It must remove bytes first.

Fixes: fe94cc290f53 ("bpf: Add IPv6 Segment Routing helpers")
Signed-off-by: Chris Mason <mason@kernel.org>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 net/core/filter.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/core/filter.c b/net/core/filter.c
index a69f6817e273..acd1e89351d6 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -7179,6 +7179,9 @@ BPF_CALL_3(bpf_lwt_seg6_adjust_srh, struct sk_buff *, skb, u32, offset,
 		return -EFAULT;
 	if (unlikely(len < 0 && (void *)((char *)ptr - len) > srh_end))
 		return -EFAULT;
+	/* The length field of the SRH has 8 bits, in units of 8 bytes. */
+	if (unlikely(len > (U8_MAX << 3) - srh_state->hdrlen))
+		return -EFAULT;
 
 	if (len > 0) {
 		ret = skb_cow_head(skb, len);
-- 
2.55.0

[PATCH bpf 19/27] bpf: Don't look for the SRH again in the End.BPF helpers

[Alexei Starovoitov]

From: Alexei Starovoitov <ast@kernel.org>

A packet can have two SRHs. Thus input_action_end_bpf() uses
IP6_FH_F_SKIP_RH to skip routing headers with segments_left 0. The helpers
bpf_lwt_seg6_store_bytes() and bpf_lwt_seg6_adjust_srh() can reallocate the
head of the skb. Thus they look for the SRH again, but without this flag.

  +------------------------+ <- the helpers: srh_state->srh
  | SRH 1, segments_left 0 |
  +------------------------+ <- input_action_end_bpf(): srh_state->srh
  | SRH 2                  |    and srh_state->hdrlen
  +------------------------+

Then the helpers check offset against the wrong limits. If a helper changes
TLVs, seg6_bpf_has_valid_srh() writes the length of SRH 2 to SRH 1.

Neither helper changes the offset of the SRH from skb->data. Calculate the
new address from this offset. To add IP6_FH_F_SKIP_RH is not sufficient,
because advance_nextseg() can set segments_left of SRH 2 to 0. The helper
bpf_lwt_seg6_action() can change the headers and still looks for the SRH.

Fixes: fbbc5bc2ab8c ("seg6: ignore routing header with segments left equal to 0")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 net/core/filter.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index acd1e89351d6..d634850a38c7 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -7024,12 +7024,13 @@ BPF_CALL_4(bpf_lwt_seg6_store_bytes, struct sk_buff *, skb, u32, offset,
 		this_cpu_ptr(&seg6_bpf_srh_states);
 	struct ipv6_sr_hdr *srh = srh_state->srh;
 	void *srh_tlvs, *srh_end, *ptr;
-	int srhoff = 0;
+	int srhoff;
 
 	lockdep_assert_held(&srh_state->bh_lock);
 	if (srh == NULL)
 		return -EINVAL;
 
+	srhoff = (unsigned char *)srh - skb->data;
 	srh_tlvs = (void *)((char *)srh + ((srh->first_segment + 1) << 4));
 	srh_end = (void *)((char *)srh + sizeof(*srh) + srh_state->hdrlen);
 
@@ -7042,8 +7043,7 @@ BPF_CALL_4(bpf_lwt_seg6_store_bytes, struct sk_buff *, skb, u32, offset,
 
 	if (unlikely(bpf_try_make_writable(skb, offset + len)))
 		return -EFAULT;
-	if (ipv6_find_hdr(skb, &srhoff, IPPROTO_ROUTING, NULL, NULL) < 0)
-		return -EINVAL;
+	/* The head may have moved. */
 	srh_state->srh = (struct ipv6_sr_hdr *)(skb->data + srhoff);
 
 	memcpy(skb->data + offset, from, len);
@@ -7162,13 +7162,14 @@ BPF_CALL_3(bpf_lwt_seg6_adjust_srh, struct sk_buff *, skb, u32, offset,
 	struct ipv6_sr_hdr *srh = srh_state->srh;
 	void *srh_end, *srh_tlvs, *ptr;
 	struct ipv6hdr *hdr;
-	int srhoff = 0;
+	int srhoff;
 	int ret;
 
 	lockdep_assert_held(&srh_state->bh_lock);
 	if (unlikely(srh == NULL))
 		return -EINVAL;
 
+	srhoff = (unsigned char *)srh - skb->data;
 	srh_tlvs = (void *)((unsigned char *)srh + sizeof(*srh) +
 			((srh->first_segment + 1) << 4));
 	srh_end = (void *)((unsigned char *)srh + sizeof(*srh) +
@@ -7200,8 +7201,7 @@ BPF_CALL_3(bpf_lwt_seg6_adjust_srh, struct sk_buff *, skb, u32, offset,
 	hdr = (struct ipv6hdr *)skb->data;
 	hdr->payload_len = htons(skb->len - sizeof(struct ipv6hdr));
 
-	if (ipv6_find_hdr(skb, &srhoff, IPPROTO_ROUTING, NULL, NULL) < 0)
-		return -EINVAL;
+	/* The bytes in front of offset have moved together with skb->data. */
 	srh_state->srh = (struct ipv6_sr_hdr *)(skb->data + srhoff);
 	srh_state->hdrlen += len;
 	srh_state->valid = false;
-- 
2.55.0

[PATCH bpf 20/27] bpf: Move the MAC header in bpf_lwt_seg6_adjust_srh()

[Alexei Starovoitov]

From: Chris Mason <mason@kernel.org>

bpf_skb_net_hdr_push() and bpf_skb_net_hdr_pop() change skb->data and
skb->mac_header by len. That is correct in a tc prog, where skb->data is
the MAC header. But bpf_lwt_seg6_adjust_srh() calls them in an End.BPF
prog, where skb->data is the network header. The MAC header does not move:

  before push  |       free       | MAC header   | IPv6 | SRH | payload
               ^ head             ^ mac_header   ^ data
  today        |       free       | MAC h| IPv6 | SRH   + len | payload
               ^ head     ^ mac_header   ^ data
  this patch   |   free   | MAC header   | IPv6 | SRH   + len | payload
               ^ head     ^ mac_header   ^ data

Users of skb_mac_header() get incorrect bytes until __dev_queue_xmit() sets
skb->mac_header again. skb_cow_head() reserves only len bytes. Thus the
16-bit offset skb->mac_header can wrap. A push also changes the "not set"
value. In both cases skb_mac_header() points after the skb.

Move the MAC header too. Pass len + skb->mac_len to skb_cow_head(). Keep
the "not set" value. The second Fixes tag is for this case. Since that
commit an End.BPF prog can get an skb with skb->mac_header not set.

Fixes: fe94cc290f53 ("bpf: Add IPv6 Segment Routing helpers")
Fixes: 20a081b7984c ("seg6: add VRF support for SRv6 End.DT6 behavior")
Signed-off-by: Chris Mason <mason@kernel.org>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 net/core/filter.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index d634850a38c7..482f8132d9ca 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -7159,6 +7159,7 @@ BPF_CALL_3(bpf_lwt_seg6_adjust_srh, struct sk_buff *, skb, u32, offset,
 {
 	struct seg6_bpf_srh_state *srh_state =
 		this_cpu_ptr(&seg6_bpf_srh_states);
+	bool mac_set = skb_mac_header_was_set(skb);
 	struct ipv6_sr_hdr *srh = srh_state->srh;
 	void *srh_end, *srh_tlvs, *ptr;
 	struct ipv6hdr *hdr;
@@ -7184,20 +7185,38 @@ BPF_CALL_3(bpf_lwt_seg6_adjust_srh, struct sk_buff *, skb, u32, offset,
 	if (unlikely(len > (U8_MAX << 3) - srh_state->hdrlen))
 		return -EFAULT;
 
+	/*
+	 * skb->data is the network header here. The MAC header is in front of
+	 * it, so bpf_skb_net_hdr_push() and bpf_skb_net_hdr_pop() do not move
+	 * it with the other headers.
+	 */
 	if (len > 0) {
-		ret = skb_cow_head(skb, len);
+		ret = skb_cow_head(skb, len + skb->mac_len);
 		if (unlikely(ret < 0))
 			return ret;
 
+		/* The push can write over the MAC header, so move it first. */
+		if (mac_set)
+			memmove(skb->data - len - skb->mac_len,
+				skb_mac_header(skb), skb->mac_len);
 		ret = bpf_skb_net_hdr_push(skb, offset, len);
 	} else {
 		ret = bpf_skb_net_hdr_pop(skb, offset, -1 * len);
+		/* The pop has added -len to skb->mac_header. */
+		if (likely(!ret) && mac_set)
+			memmove(skb->data - skb->mac_len,
+				skb_mac_header(skb) + len, skb->mac_len);
 	}
 
 	bpf_compute_data_pointers(skb);
 	if (unlikely(ret < 0))
 		return ret;
 
+	if (mac_set)
+		skb_set_mac_header(skb, -skb->mac_len);
+	else
+		skb_unset_mac_header(skb);
+
 	hdr = (struct ipv6hdr *)skb->data;
 	hdr->payload_len = htons(skb->len - sizeof(struct ipv6hdr));
 
-- 
2.55.0

[PATCH bpf 21/27] bpf: Don't leave a stale MAC header in bpf_lwt_push_ip_encap()

[Alexei Starovoitov]

From: Alexei Starovoitov <ast@kernel.org>

With BPF_LWT_ENCAP_IP, bpf_lwt_push_encap() pushes a new IP header. The new
header overwrites the MAC header of a received skb. But skb->mac_header
continues to point to the same bytes. Code that reads the MAC header after
the push, for example netfilter, gets bytes of the new header.

  before the push                        | MAC header  | old IP | payload
                                         ^ mac_header  ^ network_header
  today                            | new header        | old IP | payload
                    network_header ^     ^ mac_header
  this patch, lwt_in | MAC header  | new header        | old IP | payload
                     ^ mac_header  ^ network_header
  this patch, lwt_xmit             | new header        | old IP | payload
                    network_header ^     mac_header is not set

For lwt_in, move the MAC header in front of the new header before the
memcpy(), as __seg6_do_srh_encap() does. skb_cow_head() already reserves
len + skb->mac_len bytes.

For lwt_xmit, the output path makes a new MAC header. Thus mark
skb->mac_header as not set. This is the state of a locally generated skb.

Fixes: 52f278774e79 ("bpf: implement BPF_LWT_ENCAP_IP mode in bpf_lwt_push_encap")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 net/core/lwt_bpf.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/net/core/lwt_bpf.c b/net/core/lwt_bpf.c
index da49364ec63d..b9270a08cc6d 100644
--- a/net/core/lwt_bpf.c
+++ b/net/core/lwt_bpf.c
@@ -649,6 +649,14 @@ int bpf_lwt_push_ip_encap(struct sk_buff *skb, void *hdr, u32 len, bool ingress)
 	if (ingress)
 		skb_postpush_rcsum(skb, iph, len);
 	skb_reset_network_header(skb);
+	/*
+	 * The new header goes over the MAC header. On ingress move it first.
+	 * On egress the packet gets a new one, so forget the old one.
+	 */
+	if (ingress)
+		skb_mac_header_rebuild(skb);
+	else
+		skb_unset_mac_header(skb);
 	if (is_udp_tunnel) {
 		size_t iph_sz = ipv4 ? iph->ihl * 4 : sizeof(struct ipv6hdr);
 
-- 
2.55.0

[PATCH bpf 22/27] bpf: Reset skb->cb after an lwt prog pushes an IP header

[Alexei Starovoitov]

From: Alexei Starovoitov <ast@kernel.org>

IPCB(skb)->opt holds the offsets from the network header to the IPv4
options. But bpf_lwt_push_ip_encap() moves the network header and does not
change IPCB(skb). Then ip_forward_options() and __ip_options_echo() use the
old offsets on the pushed header. Thus the kernel corrupts the skb and the
stack. If the family changes, the kernel also reads IP6CB(skb) as
IPCB(skb), or the reverse. With CONFIG_IPV6_MIP6, it reads IPSKB_FORWARDED
as frag_max_size and adds a needless fragment header.

Reset skb->cb after the prog. bpf_lwt_push_ip_encap() cannot do it, because
bpf_prog_run_save_cb() can restore skb->cb. For IPv4 in IPv4, clear
IPCB(skb)->opt as ip_tunnel_xmit() does. If the family changes, clear the
cb of the new family but keep iif, because ip6_forward() gets the input
device from it. Behind a VRF, skb->skb_iif is the VRF device and cannot
replace iif. Do nothing for IPv6 in IPv6, as in seg6_do_srh_encap().

Do the reset in bpf_input() only after a push. Without a push,
ip_forward_options() needs the offsets. Do it in bpf_xmit() before each
reroute, because an lwt_xmit prog can also change the options. Thus after a
reroute without a push, ip_options_fragment() leaves the options in all
fragments, and ICMP errors echo none. After BPF_OK or BPF_REDIRECT the skb
goes to a device, thus bpf_xmit() does no reset. The inner IPv4 checksum
can still be wrong after ip_rcv_options() changes an option.

Fixes: 52f278774e79 ("bpf: implement BPF_LWT_ENCAP_IP mode in bpf_lwt_push_encap")
Reported-by: Xiang Mei <xmei5@asu.edu>
Suggested-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 net/core/lwt_bpf.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/net/core/lwt_bpf.c b/net/core/lwt_bpf.c
index b9270a08cc6d..3a946e8709b1 100644
--- a/net/core/lwt_bpf.c
+++ b/net/core/lwt_bpf.c
@@ -85,6 +85,32 @@ static int run_lwt_bpf(struct sk_buff *skb, struct bpf_lwt_prog *lwt,
 	return ret;
 }
 
+/*
+ * skb->cb still describes the IP header that was the first one before the
+ * prog ran. The option offsets in IPCB(skb)->opt are wrong for a pushed
+ * header, and a header of the other family needs the other layout. iif
+ * stays: behind a VRF skb->skb_iif is the L3 master and not the port.
+ * bpf_lwt_push_encap() cannot do this, because bpf_prog_run_save_cb() can
+ * restore skb->cb after the prog.
+ */
+static void bpf_lwt_reset_cb(struct sk_buff *skb, __be16 proto)
+{
+	int iif;
+
+	if (skb->protocol == proto) {
+		if (proto == htons(ETH_P_IP))
+			memset(&IPCB(skb)->opt, 0, sizeof(IPCB(skb)->opt));
+	} else if (skb->protocol == htons(ETH_P_IP)) {
+		iif = IP6CB(skb)->iif;
+		memset(IPCB(skb), 0, sizeof(*IPCB(skb)));
+		IPCB(skb)->iif = iif;
+	} else {
+		iif = IPCB(skb)->iif;
+		memset(IP6CB(skb), 0, sizeof(*IP6CB(skb)));
+		IP6CB(skb)->iif = iif;
+	}
+}
+
 static int bpf_lwt_input_reroute(struct sk_buff *skb)
 {
 	enum skb_drop_reason reason;
@@ -124,6 +150,8 @@ static int bpf_lwt_input_reroute(struct sk_buff *skb)
 static int bpf_input(struct sk_buff *skb)
 {
 	struct dst_entry *dst = skb_dst(skb);
+	__be16 proto = skb->protocol;
+	unsigned int len = skb->len;
 	struct bpf_lwt *bpf;
 	int ret;
 
@@ -132,6 +160,9 @@ static int bpf_input(struct sk_buff *skb)
 		ret = run_lwt_bpf(skb, &bpf->in, dst, NO_REDIRECT);
 		if (ret < 0)
 			return ret;
+		/* An lwt_in prog can change the length only with a push. */
+		if (skb->len != len)
+			bpf_lwt_reset_cb(skb, proto);
 		if (ret == BPF_LWT_REROUTE)
 			return bpf_lwt_input_reroute(skb);
 	}
@@ -311,6 +342,8 @@ static int bpf_xmit(struct sk_buff *skb)
 		case BPF_REDIRECT:
 			return LWTUNNEL_XMIT_DONE;
 		case BPF_LWT_REROUTE:
+			/* An lwt_xmit prog can also write to the options. */
+			bpf_lwt_reset_cb(skb, proto);
 			return bpf_lwt_xmit_reroute(skb);
 		default:
 			return ret;
-- 
2.55.0

Re: [PATCH bpf 22/27] bpf: Reset skb->cb after an lwt prog pushes an IP header

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] bpf: incomplete skb->cb reset and VRF flag loss in bpf_lwt_reset_cb()
--

commit 40c1b82f0bd0c6a0826ad31c22402c9c073712bd
Author: Alexei Starovoitov <ast@kernel.org>

bpf: Reset skb->cb after an lwt prog pushes an IP header

This commit adds bpf_lwt_reset_cb() to reset skb->cb in bpf_input() after
an lwt program pushes an encapsulation header and in bpf_xmit() before
rerouting. This avoids interpreting stale inner IP option offsets or
cross-family control block fields against the outer header.

> diff --git a/net/core/lwt_bpf.c b/net/core/lwt_bpf.c
> index b9270a08cc6d1..3a946e8709b16 100644
> --- a/net/core/lwt_bpf.c
> +++ b/net/core/lwt_bpf.c
> @@ -85,6 +85,32 @@ static int run_lwt_bpf(struct sk_buff *skb, struct bpf_lwt_prog *lwt,
>  	return ret;
>  }
>  
> +/*
> + * skb->cb still describes the IP header that was the first one before the
> + * prog ran. The option offsets in IPCB(skb)->opt are wrong for a pushed
> + * header, and a header of the other family needs the other layout. iif
> + * stays: behind a VRF skb->skb_iif is the L3 master and not the port.
> + * bpf_lwt_push_encap() cannot do this, because bpf_prog_run_save_cb() can
> + * restore skb->cb after the prog.
> + */
> +static void bpf_lwt_reset_cb(struct sk_buff *skb, __be16 proto)
> +{
> +	int iif;
> +
> +	if (skb->protocol == proto) {
> +		if (proto == htons(ETH_P_IP))
> +			memset(&IPCB(skb)->opt, 0, sizeof(IPCB(skb)->opt));
> +	} else if (skb->protocol == htons(ETH_P_IP)) {
> +		iif = IP6CB(skb)->iif;
> +		memset(IPCB(skb), 0, sizeof(*IPCB(skb)));
> +		IPCB(skb)->iif = iif;
> +	} else {
> +		iif = IPCB(skb)->iif;
> +		memset(IP6CB(skb), 0, sizeof(*IP6CB(skb)));
> +		IP6CB(skb)->iif = iif;
> +	}
> +}

[Severity: Critical]
Does bpf_lwt_reset_cb() leave several fields in IPCB(skb) and IP6CB(skb) in
an inconsistent state after encapsulation or rerouting?

1. When transitioning from IPv4 to IPv6, memset(IP6CB(skb), 0, ...) leaves
IP6CB(skb)->nhoff as 0 instead of offsetof(struct ipv6hdr, nexthdr). If
bpf_lwt_input_reroute() routes the encapsulated IPv6 packet to local
delivery, ip6_input() -> ip6_protocol_deliver_rcu() reads:

net/ipv6/ip6_input.c:ip6_protocol_deliver_rcu() {
	...
	nhoff = IP6CB(skb)->nhoff;
	if (!have_final) {
		if (!pskb_pull(skb, skb_transport_offset(skb)))
			goto discard;
		nexthdr = skb_network_header(skb)[nhoff];
	}
}

With nhoff == 0, this reads byte 0 of struct ipv6hdr (version and traffic
class) as the next header protocol instead of ipv6_hdr(skb)->nexthdr.

2. For IPv6-in-IPv6 encapsulation or rerouting (skb->protocol == proto ==
htons(ETH_P_IPV6)), bpf_lwt_reset_cb() does nothing. However, IP6CB(skb)
holds extension header offsets relative to skb_network_header(skb) (ra,
dst0, srcrt, dst1, lastopt, nhoff, dsthao, srhoff) as well as flags
(IP6SKB_HOPBYHOP, IP6SKB_ROUTERALERT, IP6SKB_FRAGMENTED) and frag_max_size
parsed from the inner IPv6 packet before bpf_input() or bpf_xmit() ran.

After bpf_lwt_push_ip_encap() moves skb->network_header to the newly pushed
outer IPv6 header (or an lwt_xmit program modifies headers and returns
BPF_LWT_REROUTE):

- With CONFIG_IPV6_MIP6, if icmpv6_send() is triggered on the outer packet,
  mip6_addr_swap() uses the stale IP6CB(skb)->dsthao offset relative to the
  new outer network header:

  net/ipv6/icmp.c:mip6_addr_swap() {
	...
	if (opt->dsthao) {
		off = ipv6_find_tlv(skb, opt->dsthao, IPV6_TLV_HAO);
		if (likely(off >= 0)) {
			hao = (struct ipv6_destopt_hao *)
					(skb_network_header(skb) + off);
			swap(iph->saddr, hao->addr);
		}
	}
  }

  Can this corrupt the packet or swap 16 bytes out of bounds?

- On local delivery via ip6_protocol_deliver_rcu() or
  ip6_datagram_recv_specific_ctl(), the stack will index from the new
  network header using the inner packet's nhoff, lastopt, dst0, dst1, and
  srcrt offsets.

- In ip6_forward() and __ip6_finish_output(), stale IP6SKB_ROUTERALERT or
  frag_max_size from the inner packet will divert or fragment the outer
  packet.

3. When a packet arrives on a port enslaved to a VRF device, vrf_ip_rcv()
and vrf_ip6_rcv() set IPSKB_L3SLAVE or IP6SKB_L3SLAVE in cb->flags while
keeping the slave port ifindex in cb->iif and setting skb->skb_iif to the
VRF master ifindex. Because bpf_lwt_reset_cb() clears flags without
translating IPSKB_L3SLAVE and IP6SKB_L3SLAVE across family transitions,
inet6_iif() and inet_sdif() lose the VRF association:

include/linux/ipv6.h:inet6_iif() {
	bool l3_slave = ipv6_l3mdev_skb(IP6CB(skb)->flags);

	return l3_slave ? skb->skb_iif : IP6CB(skb)->iif;
}

4. For IPv4-in-IPv4, only IPCB(skb)->opt is zeroed, leaving
IPCB(skb)->frag_max_size and non-VRF flags from the inner packet set, which
causes __ip_finish_output() to fragment the outer packet to the inner
packet's reassembled fragment size.

Should bpf_lwt_reset_cb() reset the full control block for both families
 while preserving iif, translating IPSKB_L3SLAVE / IP6SKB_L3SLAVE, and
initializing IP6CB(skb)->nhoff = offsetof(struct ipv6hdr, nexthdr), similar
to decap_and_validate() in net/ipv6/seg6_local.c?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261101190535.1454146-1-alexei.starovoitov@gmail.com?part=22

[PATCH bpf 23/27] bpf: Require BPF_LWT_REROUTE when an lwt_in prog changes the protocol

[Alexei Starovoitov]

From: Alexei Starovoitov <ast@kernel.org>

When an lwt_in prog returns BPF_OK, bpf_input() calls orig_input(), the
input handler of the route. This handler is for one IP version and needs a
dst of the same family. But BPF_LWT_ENCAP_IP lets the prog push a header of
the other IP version and thus change skb->protocol. Then the handler reads
and changes a header of the other IP version. The push of an SRH also
replaces the dst, because seg6_lookup_nexthop() puts an rt6_info into the
skb. Then ip_forward() uses the rt6_info as a struct rtable.

Drop the skb with -EINVAL when skb->protocol changes, as bpf_xmit() does.
Drop it also when the family of the dst changes, because skb->protocol does
not always show this change. Today no such skb leaves as the prog made it.
A prog works as before if it returns BPF_LWT_REROUTE, or if it pushes an
SRH on an IPv6 route.

The commit in the first Fixes tag lets a prog change skb->protocol. The
commit in the second tag adds the check to bpf_xmit() only.

Fixes: 52f278774e79 ("bpf: implement BPF_LWT_ENCAP_IP mode in bpf_lwt_push_encap")
Fixes: 3bd0b15281af ("bpf: add handling of BPF_LWT_REROUTE to lwt_bpf.c")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 net/core/lwt_bpf.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/net/core/lwt_bpf.c b/net/core/lwt_bpf.c
index 3a946e8709b1..60f4eda7dae9 100644
--- a/net/core/lwt_bpf.c
+++ b/net/core/lwt_bpf.c
@@ -165,6 +165,16 @@ static int bpf_input(struct sk_buff *skb)
 			bpf_lwt_reset_cb(skb, proto);
 		if (ret == BPF_LWT_REROUTE)
 			return bpf_lwt_input_reroute(skb);
+		/*
+		 * orig_input() is for the old protocol only, see bpf_xmit().
+		 * It also needs a dst of the old family. BPF_LWT_ENCAP_SEG6
+		 * puts an IPv6 dst into the skb.
+		 */
+		if (skb->protocol != proto ||
+		    skb_dst(skb)->ops->family != dst->ops->family) {
+			kfree_skb(skb);
+			return -EINVAL;
+		}
 	}
 
 	if (unlikely(!dst->lwtstate->orig_input)) {
-- 
2.55.0

[PATCH bpf 24/27] bpf: Don't let lwt_in progs call into IPv6 when it is disabled

[Alexei Starovoitov]

From: Alexei Starovoitov <ast@kernel.org>

With ipv6.disable=1, inet6_init() returns before it registers any pernet
subsystem. Thus net->ipv6.devconf_all and net->ipv6.seg6_data stay NULL.
IS_ENABLED(CONFIG_IPV6) only says that the code is built in.
bpf_lwt_input_reroute() and bpf_push_seg6_encap() call into IPv6, but do
not test ipv6_mod_enabled(). An lwt_in prog also runs with ipv6.disable=1,
and skb->protocol == htons(ETH_P_IPV6) does not prove that IPv6 is enabled.
Thus the kernel dereferences NULL and crashes.

Return -EAFNOSUPPORT before these calls if ipv6_mod_enabled() is false.
BPF_LWT_REROUTE then drops an IPv6 skb. With IPv6 enabled nothing changes.

The first Fixes tag is for bpf_push_seg6_encap(), which never had this
test. The second tag is for bpf_lwt_input_reroute(). Until that commit it
used ipv6_stub, which returned -EAFNOSUPPORT with ipv6.disable=1.

Fixes: fe94cc290f53 ("bpf: Add IPv6 Segment Routing helpers")
Fixes: ad84b1eefe28 ("bpf: remove ipv6_bpf_stub completely and use direct function calls")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 net/core/filter.c  | 4 ++++
 net/core/lwt_bpf.c | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index 482f8132d9ca..08168ea381c2 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -6924,6 +6924,10 @@ static int bpf_push_seg6_encap(struct sk_buff *skb, u32 type, void *hdr, u32 len
 	int err;
 	struct ipv6_sr_hdr *srh = (struct ipv6_sr_hdr *)hdr;
 
+	/* With ipv6.disable=1 the netns has no seg6 data and no IPv6 tables. */
+	if (!ipv6_mod_enabled())
+		return -EAFNOSUPPORT;
+
 	if (!seg6_validate_srh(srh, len, false))
 		return -EINVAL;
 
diff --git a/net/core/lwt_bpf.c b/net/core/lwt_bpf.c
index 60f4eda7dae9..774fad4e9fbf 100644
--- a/net/core/lwt_bpf.c
+++ b/net/core/lwt_bpf.c
@@ -128,7 +128,7 @@ static int bpf_lwt_input_reroute(struct sk_buff *skb)
 		dev_put(dev);
 	} else if (skb->protocol == htons(ETH_P_IPV6)) {
 		skb_dst_drop(skb);
-		if (IS_ENABLED(CONFIG_IPV6)) {
+		if (ipv6_mod_enabled()) {
 			ip6_route_input(skb);
 			err = skb_dst(skb)->error;
 		} else {
-- 
2.55.0

[PATCH bpf 25/27] bpf: Move the transport header with the moved headers

[Alexei Starovoitov]

From: Chris Mason <mason@kernel.org>

bpf_skb_net_hdr_pop() removes len bytes at offset off.
bpf_skb_net_hdr_push() does the opposite.

           skb->data       off
  before   | A             | removed | B            |
  after              | A             | B            |
                     skb->data       off

mac_header and network_header move with A. transport_header moves only when
it is equal to network_header. inner_transport_header never moves. This is
correct only for a header in B. In bpf_lwt_seg6_adjust_srh()
transport_header is in A. bpf_skb_adjust_room() can remove both transport
headers. With an incorrect header offset, the stack can drop the skb or
read outside of it.

Also move transport_header with A when it is in A. Set it to offset off
when it is in the removed bytes. In BPF_ADJ_ROOM_MAC mode it is then equal
to network_header. Do not unset it, because skb_gso_validate_mac_len() uses
it without a check. After a removal, set inner_transport_header to off when
it is not in B and skb->encapsulation is set. A has only outer headers.

Fixes: fe94cc290f53 ("bpf: Add IPv6 Segment Routing helpers")
Fixes: 868d523535c2 ("bpf: add bpf_skb_adjust_room encap flags")
Signed-off-by: Chris Mason <mason@kernel.org>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 net/core/filter.c | 37 +++++++++++++++++++++++++++++++------
 1 file changed, 31 insertions(+), 6 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index 08168ea381c2..b5534dac9e6d 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3402,9 +3402,25 @@ static int bpf_skb_generic_pop(struct sk_buff *skb, u32 off, u32 len)
 	return 0;
 }
 
+/*
+ * bpf_skb_net_hdr_push() inserts bytes at @off, bpf_skb_net_hdr_pop() removes
+ * bytes at @off. Both move the bytes in front of @off together with skb->data,
+ * and leave the rest of the packet where it is. Return true if the transport
+ * header is in the part that moves. A transport header that is equal to the
+ * network header stays equal to it.
+ */
+static bool bpf_skb_trans_hdr_moves(const struct sk_buff *skb, u32 off)
+{
+	if (skb->transport_header == skb->network_header)
+		return true;
+
+	return skb_transport_header_was_set(skb) &&
+	       (u32)skb_transport_offset(skb) < off;
+}
+
 static int bpf_skb_net_hdr_push(struct sk_buff *skb, u32 off, u32 len)
 {
-	bool trans_same = skb->transport_header == skb->network_header;
+	bool trans_moves = bpf_skb_trans_hdr_moves(skb, off);
 	int ret;
 
 	/* There's no need for __skb_push()/__skb_pull() pair to
@@ -3415,8 +3431,8 @@ static int bpf_skb_net_hdr_push(struct sk_buff *skb, u32 off, u32 len)
 	if (likely(!ret)) {
 		skb->mac_header -= len;
 		skb->network_header -= len;
-		if (trans_same)
-			skb->transport_header = skb->network_header;
+		if (trans_moves)
+			skb->transport_header -= len;
 	}
 
 	return ret;
@@ -3424,7 +3440,7 @@ static int bpf_skb_net_hdr_push(struct sk_buff *skb, u32 off, u32 len)
 
 static int bpf_skb_net_hdr_pop(struct sk_buff *skb, u32 off, u32 len)
 {
-	bool trans_same = skb->transport_header == skb->network_header;
+	bool trans_moves = bpf_skb_trans_hdr_moves(skb, off);
 	int ret;
 
 	/* Same here, __skb_push()/__skb_pull() pair not needed. */
@@ -3432,8 +3448,17 @@ static int bpf_skb_net_hdr_pop(struct sk_buff *skb, u32 off, u32 len)
 	if (likely(!ret)) {
 		skb->mac_header += len;
 		skb->network_header += len;
-		if (trans_same)
-			skb->transport_header = skb->network_header;
+		if (trans_moves) {
+			skb->transport_header += len;
+		} else if (skb_transport_header_was_set(skb) &&
+			   skb_transport_offset(skb) < (int)off) {
+			/* It was in the removed bytes or before skb->data. */
+			skb_set_transport_header(skb, off);
+		}
+		/* Do not leave the inner transport header in front of @off. */
+		if (skb->encapsulation &&
+		    skb_inner_transport_offset(skb) < (int)off)
+			skb_set_inner_transport_header(skb, off);
 	}
 
 	return ret;
-- 
2.55.0

[PATCH bpf 26/27] bpf: Use the inner transport header after a decap

[Alexei Starovoitov]

From: Chris Mason <mason@kernel.org>

If skb->encapsulation is set, qdisc_pkt_len_segs_init() reads the TCP
header of a GSO packet at skb->inner_transport_header. Otherwise it uses
skb->transport_header. A decap with BPF_F_ADJ_ROOM_DECAP_L4_{GRE,UDP}
removes the outer transport header. It clears skb->encapsulation if no
tunnel flag remains in gso_type. The result in BPF_ADJ_ROOM_MAC mode is:

  before  | MAC | outer IP | GRE | inner IP | TCP | payload
          transport_header ^                ^ inner_transport_header
  after                    | MAC | inner IP | TCP | payload
                transport_header ^          ^ inner_transport_header

Then qdisc_pkt_len_segs_init() reads the inner IP header as a TCP header.
Thus the kernel can drop a valid packet with SKB_DROP_REASON_SKB_BAD_GSO.

Copy skb->inner_transport_header to skb->transport_header when a decap
clears skb->encapsulation. Keep skb->transport_header if it is larger. This
occurs if nothing sets the inner field. At tc ingress nothing changes for
IPIP or GRO packets. Both fields are at the TCP header there.

Fixes: ec20dee2f2c4 ("bpf: Clear decap state on skb_adjust_room shrink path")
Signed-off-by: Chris Mason <mason@kernel.org>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 net/core/filter.c | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index b5534dac9e6d..21fcdc7cd950 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3769,6 +3769,25 @@ static int bpf_skb_net_grow(struct sk_buff *skb, u32 off, u32 len_diff,
 	return 0;
 }
 
+/*
+ * Code that went by the inner transport header while skb->encapsulation was set
+ * goes by the transport header from now on. So take the inner one over, unless
+ * it is the smaller one, as it is if nobody has set it. Do not leave the result
+ * in front of @off, where the bytes were removed.
+ */
+static void bpf_skb_clear_encap(struct sk_buff *skb, u32 off)
+{
+	int thoff;
+
+	if (!skb->encapsulation)
+		return;
+
+	thoff = max(skb->inner_transport_header, skb->transport_header) -
+		skb_headroom(skb);
+	skb_set_transport_header(skb, max_t(int, thoff, off));
+	skb->encapsulation = 0;
+}
+
 static int bpf_skb_net_shrink(struct sk_buff *skb, u32 off, u32 len_diff,
 			      u64 flags)
 {
@@ -3850,8 +3869,7 @@ static int bpf_skb_net_shrink(struct sk_buff *skb, u32 off, u32 len_diff,
 						  SKB_GSO_IPXIP4 |
 						  SKB_GSO_IPXIP6 |
 						  SKB_GSO_ESP)))
-				if (skb->encapsulation)
-					skb->encapsulation = 0;
+				bpf_skb_clear_encap(skb, off);
 		}
 
 		/* Header must be checked, and gso_segs recomputed. */
@@ -3859,10 +3877,9 @@ static int bpf_skb_net_shrink(struct sk_buff *skb, u32 off, u32 len_diff,
 		shinfo->gso_segs = 0;
 	} else {
 		/* For non-GSO packets, clear encapsulation if decap flags are set */
-		if ((flags & (BPF_F_ADJ_ROOM_DECAP_L4_MASK |
-			      BPF_F_ADJ_ROOM_DECAP_IPXIP_MASK)) &&
-		    skb->encapsulation)
-			skb->encapsulation = 0;
+		if (flags & (BPF_F_ADJ_ROOM_DECAP_L4_MASK |
+			     BPF_F_ADJ_ROOM_DECAP_IPXIP_MASK))
+			bpf_skb_clear_encap(skb, off);
 	}
 
 	return 0;
-- 
2.55.0

Re: [PATCH bpf 26/27] bpf: Use the inner transport header after a decap

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] bpf: mishandled unset ~0U transport header in bpf_skb_clear_encap()
--

commit 907dfd6423a9766152f03a831bcc864117a68b99
Author: Chris Mason <mason@kernel.org>

bpf: Use the inner transport header after a decap

This commit adds bpf_skb_clear_encap() to update skb->transport_header from
skb->inner_transport_header when decapsulation in bpf_skb_net_shrink() clears
skb->encapsulation.

> diff --git a/net/core/filter.c b/net/core/filter.c
> index b5534dac9e6dc..21fcdc7cd950d 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -3769,6 +3769,25 @@ static int bpf_skb_net_grow(struct sk_buff *skb, u32 off, u32 len_diff,
>  	return 0;
>  }
>  
> +/*
> + * Code that went by the inner transport header while skb->encapsulation was set
> + * goes by the transport header from now on. So take the inner one over, unless
> + * it is the smaller one, as it is if nobody has set it. Do not leave the result
> + * in front of @off, where the bytes were removed.
> + */
> +static void bpf_skb_clear_encap(struct sk_buff *skb, u32 off)
> +{
> +	int thoff;
> +
> +	if (!skb->encapsulation)
> +		return;
> +
> +	thoff = max(skb->inner_transport_header, skb->transport_header) -
> +		skb_headroom(skb);
> +	skb_set_transport_header(skb, max_t(int, thoff, off));
> +	skb->encapsulation = 0;
> +}

[Severity: High]
Can max(skb->inner_transport_header, skb->transport_header) in
bpf_skb_clear_encap() in net/core/filter.c select the unset sentinel (~0U)
instead of a valid transport header offset?

In include/linux/skbuff.h, an unset transport header is represented by
(__u16)~0U (0xffff):

include/linux/skbuff.h:skb_transport_header_was_set() {
	return skb->transport_header != (typeof(skb->transport_header))~0U;
}

Because 0xffff is the largest possible __u16 value rather than the smallest,
taking max(skb->inner_transport_header, skb->transport_header) will pick
0xffff whenever either field holds the unset sentinel:

1. If skb->transport_header is unset (0xffff) while
   skb->inner_transport_header holds a valid offset (for example, after
   gro_cells_receive() calls skb_unset_transport_header(), or after
   encapsulating via bpf_skb_net_grow() with BPF_F_ADJ_ROOM_ENCAP_L3_IPV4/IPV6
   without L4 flags), max() chooses 0xffff and overwrites
   skb->transport_header with 0xffff instead of copying
   skb->inner_transport_header.

2. If an skb starts with skb->transport_header unset (0xffff) and is
   encapsulated with BPF_F_ADJ_ROOM_ENCAP_L4_GRE or
   BPF_F_ADJ_ROOM_ENCAP_L4_UDP, bpf_skb_net_grow() copies 0xffff into
   skb->inner_transport_header while setting skb->transport_header to the
   outer L4 header:

net/core/filter.c:bpf_skb_net_grow() {
	if (encap) {
		...
		inner_trans = skb->transport_header;
	}
	...
	if (encap) {
		...
		skb->inner_transport_header = inner_trans;
		...
		if (flags & BPF_F_ADJ_ROOM_ENCAP_L4_GRE ||
		    flags & BPF_F_ADJ_ROOM_ENCAP_L4_UDP) {
			...
			skb_set_transport_header(skb, mac_len + nh_len);
		}
}

   When that packet is later decapsulated in bpf_skb_net_shrink(),
   bpf_skb_net_hdr_pop() clamps the outer skb->transport_header to off, while
   skb_inner_transport_offset(skb) is 0xffff - skb_headroom(skb) >= (int)off
   so skb->inner_transport_header stays 0xffff:

net/core/filter.c:bpf_skb_net_hdr_pop() {
		} else if (skb_transport_header_was_set(skb) &&
			   skb_transport_offset(skb) < (int)off) {
			/* It was in the removed bytes or before skb->data. */
			skb_set_transport_header(skb, off);
		}
		/* Do not leave the inner transport header in front of @off. */
		if (skb->encapsulation &&
		    skb_inner_transport_offset(skb) < (int)off)
			skb_set_inner_transport_header(skb, off);
}

   Then bpf_skb_clear_encap() evaluates max(0xffff, skb->transport_header) and
   sets skb->transport_header back to 0xffff.

For a GSO packet where bpf_skb_net_shrink() sets SKB_GSO_DODGY, resets
gso_segs = 0, and clears skb->encapsulation, leaving skb->transport_header at
0xffff causes qdisc_pkt_len_segs_init() to skip GSO segment calculation and
causes skb_gso_validate_mac_len() -> skb_gso_transport_seglen() ->
tcp_hdrlen() -> skb_transport_header() to trigger
DEBUG_NET_WARN_ON_ONCE(!skb_transport_header_was_set(skb)) and read from
skb->head + 0xffff.

Should bpf_skb_clear_encap() check whether the transport headers are set
(or ignore values equal to (__u16)~0U) before comparing them?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261101190535.1454146-1-alexei.starovoitov@gmail.com?part=26

[PATCH bpf 27/27] bpf: Check the offset in bpf_skb_generic_{push,pop}()

[Alexei Starovoitov]

From: Chris Mason <mason@kernel.org>

To make room for len bytes at offset off, bpf_skb_generic_push() moves the
first off bytes and clears len bytes after them. It assumes that the first
off bytes are in the linear part. But in bpf_skb_adjust_room() off can be
the end of the IP header, and this header can be in a frag. Then the
function reads and writes after skb->tail, and can damage skb_shared_info.

bpf_skb_generic_pop() checks off + len, but this u32 sum can wrap to a
small value. bpf_skb_adjust_room() and bpf_skb_change_proto() calculate off
from skb_mac_header_len(). This length is a very large u32 when
network_header < mac_header. Then the check can pass, and memmove() reads
and writes far outside the skb.

Return -ENOMEM from both functions, like the present check, when
off > skb->len. Thus the sum cannot wrap. In bpf_skb_generic_push() also
pull the first off bytes into the linear part. Now bpf_skb_adjust_room()
fails when a prog grows an skb that ends before off. network_header stays
wrong.

Fixes: 6578171a7ff0 ("bpf: add bpf_skb_change_proto helper")
Fixes: 2be7e212d541 ("bpf: add bpf_skb_adjust_room helper")
Signed-off-by: Chris Mason <mason@kernel.org>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 net/core/filter.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index 21fcdc7cd950..9245009d9f1e 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3369,6 +3369,10 @@ static const struct bpf_func_proto bpf_skb_vlan_pop_proto = {
 
 static int bpf_skb_generic_push(struct sk_buff *skb, u32 off, u32 len)
 {
+	/* The bytes in front of @off are moved with skb->data. */
+	if (unlikely(off > skb->len || !pskb_may_pull(skb, off)))
+		return -ENOMEM;
+
 	/* Caller already did skb_cow() with meta_len+len as headroom,
 	 * so no need to do it here.
 	 */
@@ -3389,9 +3393,10 @@ static int bpf_skb_generic_pop(struct sk_buff *skb, u32 off, u32 len)
 	void *old_data;
 
 	/* skb_ensure_writable() is not needed here, as we're
-	 * already working on an uncloned skb.
+	 * already working on an uncloned skb. The first test
+	 * keeps off + len from wrapping.
 	 */
-	if (unlikely(!pskb_may_pull(skb, off + len)))
+	if (unlikely(off > skb->len || !pskb_may_pull(skb, off + len)))
 		return -ENOMEM;
 
 	old_data = skb->data;
-- 
2.55.0

Re: [PATCH bpf 27/27] bpf: Check the offset in bpf_skb_generic_{push,pop}()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] bpf: u32 wraparound of off in bpf_skb_adjust_room() bypasses bounds check
--

commit 294bb6515ed77f7d0955cf5e521eacaeed3e3e77
Author: Chris Mason <mason@kernel.org>

bpf: Check the offset in bpf_skb_generic_{push,pop}()

This commit adds bounds checks in bpf_skb_generic_push() and
bpf_skb_generic_pop() to ensure off does not exceed skb->len and pulls the
first off bytes into the linear part of the skb before moving data.

> diff --git a/net/core/filter.c b/net/core/filter.c
> index 21fcdc7cd950d..9245009d9f1eb 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -3369,6 +3369,10 @@ static const struct bpf_func_proto bpf_skb_vlan_pop_proto = {
>  
>  static int bpf_skb_generic_push(struct sk_buff *skb, u32 off, u32 len)
>  {
> +	/* The bytes in front of @off are moved with skb->data. */
> +	if (unlikely(off > skb->len || !pskb_may_pull(skb, off)))
> +		return -ENOMEM;
> +
>  	/* Caller already did skb_cow() with meta_len+len as headroom,
>  	 * so no need to do it here.
>  	 */
> @@ -3389,9 +3393,10 @@ static int bpf_skb_generic_pop(struct sk_buff *skb, u32 off, u32 len)
>  	void *old_data;
>  
>  	/* skb_ensure_writable() is not needed here, as we're
> -	 * already working on an uncloned skb.
> +	 * already working on an uncloned skb. The first test
> +	 * keeps off + len from wrapping.
>  	 */
> -	if (unlikely(!pskb_may_pull(skb, off + len)))
> +	if (unlikely(off > skb->len || !pskb_may_pull(skb, off + len)))
>  		return -ENOMEM;

[Severity: High]
Can off wrap in bpf_skb_adjust_room() before reaching bpf_skb_generic_push()
or bpf_skb_generic_pop()?

In net/core/filter.c, bpf_skb_adjust_room() computes off as:

	off = skb_mac_header_len(skb);
	switch (mode) {
	case BPF_ADJ_ROOM_NET:
		off += bpf_skb_net_base_len(skb);
		break;

When skb->network_header < skb->mac_header by 1 to 20 bytes (for ETH_P_IP) or
1 to 40 bytes (for ETH_P_IPV6), skb_mac_header_len(skb) returns a u32 near
UINT_MAX (0xffffffd8..0xffffffff).

When mode is BPF_ADJ_ROOM_NET, adding bpf_skb_net_base_len(skb) (20 or 40)
wraps the 32-bit unsigned off to a small value in 0..39 before
bpf_skb_net_grow() or bpf_skb_net_shrink() calls bpf_skb_generic_push() or
bpf_skb_generic_pop().

Because off has already wrapped to 0..39, the off > skb->len check in
bpf_skb_generic_push() and bpf_skb_generic_pop() passes whenever
skb->len >= off. When growing with encapsulation flags in bpf_skb_net_grow(),
u16 mac_len = skb->network_header - skb->mac_header also wraps to
65496..65535, and skb_set_network_header(skb, mac_len) sets
skb->network_header far past the end of the packet.

Would it make sense to validate skb->network_header >= skb->mac_header (or
check skb_mac_header_len(skb) before adding bpf_skb_net_base_len(skb)) in
bpf_skb_adjust_room()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261101190535.1454146-1-alexei.starovoitov@gmail.com?part=27

Re: [PATCH bpf 00/27] bpf: Fixes for the verifier, arena, skb and lwt helpers

[patchwork-bot+netdevbpf]

Hello:

This series was applied to bpf/bpf.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Sun,  1 Nov 2026 19:05:08 +0000 you wrote:
> From: Alexei Starovoitov <ast@kernel.org>
> 
> These patches fix bugs in BPF:
> 
>   patches  area
>   -------  ----------------------------------------------------------------
>   01-02    stack traces: the limit of the trace length
>   03-07    arena: the range of mmap() and of bpf_arena_free_pages()
>   08-12    arguments of kfuncs and of extension progs
>   13       cgroup detach: the permission check
>   14       cgroup skb progs: the network offset
>   15       bpf_skb_adjust_room(): the limit of gso_size
>   16-24    lwt and seg6local End.BPF progs: pushed headers, skb->cb, dst
>   25-27    skb offsets when a helper adds or removes room
> 
> [...]

Here is the summary with links:
  - [bpf,01/27] bpf: Clamp the trace length in __bpf_get_stack() again
    https://git.kernel.org/bpf/bpf/c/007fd460798b
  - [bpf,02/27] bpf: Clamp the trace length before the skip test in stackid_init()
    https://git.kernel.org/bpf/bpf/c/a08ba2ab79df
  - [bpf,03/27] bpf: Refuse to map an arena at address 0
    https://git.kernel.org/bpf/bpf/c/ca580b034380
  - [bpf,04/27] bpf: Don't let bpf_arena_free_pages() see a half-set user range
    https://git.kernel.org/bpf/bpf/c/52498f14c2d2
  - [bpf,05/27] bpf: Refuse an arena mmap() longer than max_entries pages
    https://git.kernel.org/bpf/bpf/c/1e01345b87f1
  - [bpf,06/27] bpf: Remove the WARN from arena_get_unmapped_area()
    https://git.kernel.org/bpf/bpf/c/15dedd4b2592
  - [bpf,07/27] selftests/bpf: Test the length and the address of an arena mmap()
    https://git.kernel.org/bpf/bpf/c/14d3e5bf9ffe
  - [bpf,08/27] bpf: Reject memory for kfunc projection type args
    https://git.kernel.org/bpf/bpf/c/f89ebf07496b
  - [bpf,09/27] bpf: Don't turn struct arguments of KF_ACQUIRE kfuncs into memory
    https://git.kernel.org/bpf/bpf/c/07684726d39e
  - [bpf,10/27] bpf: Don't turn kfunc arguments of an empty struct type into memory
    https://git.kernel.org/bpf/bpf/c/ed02b9cc1d37
  - [bpf,11/27] bpf: Compare the arguments of an extension with those of its target
    https://git.kernel.org/bpf/bpf/c/faea3cda41d3
  - [bpf,12/27] selftests/bpf: Test the arguments of an extension prog
    https://git.kernel.org/bpf/bpf/c/9abda4d8fc0e
  - [bpf,13/27] bpf: Require prog fd or capability for cgroup detach
    https://git.kernel.org/bpf/bpf/c/2962329f7c86
  - [bpf,14/27] bpf, cgroup: Keep the skb network offset signed
    https://git.kernel.org/bpf/bpf/c/3f9780d62dd1
  - [bpf,15/27] bpf: Don't let bpf_skb_adjust_room() raise gso_size to GSO_BY_FRAGS
    https://git.kernel.org/bpf/bpf/c/6d8118937ba4
  - [bpf,16/27] bpf: Reject non-IPv6 packets in BPF_LWT_ENCAP_SEG6
    https://git.kernel.org/bpf/bpf/c/58e39f8d58b1
  - [bpf,17/27] bpf: Set a dst for test runs of lwt_in progs
    https://git.kernel.org/bpf/bpf/c/c1ca06e48e88
  - [bpf,18/27] bpf: Limit the SRH length in bpf_lwt_seg6_adjust_srh()
    https://git.kernel.org/bpf/bpf/c/ac159f39b252
  - [bpf,19/27] bpf: Don't look for the SRH again in the End.BPF helpers
    https://git.kernel.org/bpf/bpf/c/45bcc58d5767
  - [bpf,20/27] bpf: Move the MAC header in bpf_lwt_seg6_adjust_srh()
    https://git.kernel.org/bpf/bpf/c/0fcb3f11a93d
  - [bpf,21/27] bpf: Don't leave a stale MAC header in bpf_lwt_push_ip_encap()
    https://git.kernel.org/bpf/bpf/c/701ffd202b9e
  - [bpf,22/27] bpf: Reset skb->cb after an lwt prog pushes an IP header
    https://git.kernel.org/bpf/bpf/c/b3ad42f61832
  - [bpf,23/27] bpf: Require BPF_LWT_REROUTE when an lwt_in prog changes the protocol
    https://git.kernel.org/bpf/bpf/c/d9618a619695
  - [bpf,24/27] bpf: Don't let lwt_in progs call into IPv6 when it is disabled
    https://git.kernel.org/bpf/bpf/c/95f1d5e4abb7
  - [bpf,25/27] bpf: Move the transport header with the moved headers
    https://git.kernel.org/bpf/bpf/c/a9d08fe60fa7
  - [bpf,26/27] bpf: Use the inner transport header after a decap
    https://git.kernel.org/bpf/bpf/c/33be492c2e23
  - [bpf,27/27] bpf: Check the offset in bpf_skb_generic_{push,pop}()
    https://git.kernel.org/bpf/bpf/c/ee36c9aa44ef

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html