Skip to content

Commit 5132115

Browse files
author
Alexei Starovoitov
committed
Merge branch 'minimize-annotations-for-arena-programs'
Emil Tsalapatis says: ==================== Minimize annotations for arena programs BPF programs must currently include code to address two limitations of function signatures that include arena types. First, arena arguments must be annotated with __arg_arena in the function signature in addition to __arena. Second, it is currently not allowed to return an arena pointer from a subprog, even though it is safe to do so. These limitations require extra annotations and typecasts respectively, and have proven sources of confusion to programmers. The patchset improves arena-related function signatures in two ways. First, it removes the need for __arg_arena in function signatures. Second, it allows subprogs to directly return arena pointers to their caller. To do this we add a new type tag to the existing __arena annotation. The annotation is currently an alias for __attribute__((address_space(1))), which is not discoverable from BTF alone and so cannot be used to determine whether a pointer variable is an arena pointer during verification. With the new type tag, we can determine whether either the arguments and or the return value of a function belong in an arena. We test the new code by modifying libarena to take advantage of these relaxed limitations. CHANGELOG ========= v2 -> v3 (https://lore.kernel.org/bpf/20260530002259.4505-1-emil@etsalapatis.com/) - Added Acks by Eduard - Complete the __arg_arena removal by removing them from htab (Alexei) - Add a test in verifier_arena_globals1.c to confirm the new __arena attribute works as expected in function argument and return types - Reject type tags on non-pointer types, currently only possible in handcrafted BTF (Eduard) - Undo inaccurate change on verifier comment (AI) - Fix error return value for invalid BTF return types during BTF parsing (Eduard) v1 -> v2 (lore.kernel.org/bpf/20260527071457.4598-1-emil@etsalapatis.com/) - Rebased to fix conflict - Removed the typedef foo * foo_t typedefs. Those were necessary to avoid annotating each instance of the type with __arena. The new version of the patch instead removes typedefs and uses __arena everywhere directly (see patch 4/5 for more details). - Reorganized the patchset to frontload all kernel-side changes and place the libarena changes at the end. ==================== Link: https://patch.msgid.link/20260602004120.17087-1-emil@etsalapatis.com Signed-off-by: Alexei Starovoitov <ast@kernel.org>
2 parents b6aa0ab + 9fd5bf9 commit 5132115

17 files changed

Lines changed: 298 additions & 162 deletions

File tree

kernel/bpf/btf.c

Lines changed: 131 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -7802,6 +7802,120 @@ enum btf_arg_tag {
78027802
ARG_TAG_ARENA = BIT_ULL(5),
78037803
};
78047804

7805+
static int btf_scan_decl_tags(struct bpf_verifier_env *env,
7806+
const struct btf *btf,
7807+
const struct btf_type *fn_t,
7808+
u32 arg_idx, bool is_global, u32 *tags)
7809+
{
7810+
int id = btf_named_start_id(btf, false) - 1;
7811+
7812+
/*
7813+
* The 'arg:<tag>' decl_tag takes precedence over the derivation
7814+
* of the register type from the BTF type itself.
7815+
*/
7816+
while ((id = btf_find_next_decl_tag(btf, fn_t, arg_idx, "arg:", id)) > 0) {
7817+
const struct btf_type *tag_t = btf_type_by_id(btf, id);
7818+
const char *tag = __btf_name_by_offset(btf, tag_t->name_off) + 4;
7819+
7820+
/* disallow arg tags in static subprogs */
7821+
if (!is_global) {
7822+
bpf_log(&env->log,
7823+
"arg#%d type tag is not supported in static functions\n",
7824+
arg_idx);
7825+
return -EOPNOTSUPP;
7826+
}
7827+
7828+
if (strcmp(tag, "ctx") == 0) {
7829+
*tags |= ARG_TAG_CTX;
7830+
} else if (strcmp(tag, "trusted") == 0) {
7831+
*tags |= ARG_TAG_TRUSTED;
7832+
} else if (strcmp(tag, "untrusted") == 0) {
7833+
*tags |= ARG_TAG_UNTRUSTED;
7834+
} else if (strcmp(tag, "nonnull") == 0) {
7835+
*tags |= ARG_TAG_NONNULL;
7836+
} else if (strcmp(tag, "nullable") == 0) {
7837+
*tags |= ARG_TAG_NULLABLE;
7838+
} else if (strcmp(tag, "arena") == 0) {
7839+
*tags |= ARG_TAG_ARENA;
7840+
} else {
7841+
bpf_log(&env->log, "arg#%d has unsupported set of tags\n", arg_idx);
7842+
return -EOPNOTSUPP;
7843+
}
7844+
}
7845+
if (id != -ENOENT) {
7846+
bpf_log(&env->log, "arg#%d type tag fetching failure: %d\n", arg_idx, id);
7847+
return id;
7848+
}
7849+
7850+
return 0;
7851+
}
7852+
7853+
static int btf_scan_type_tags(struct bpf_verifier_env *env,
7854+
const struct btf *btf, u32 type_id,
7855+
u32 *tags)
7856+
{
7857+
const struct btf_type *t;
7858+
7859+
/* Find the first pointer type in the chain. */
7860+
t = btf_type_skip_modifiers(btf, type_id, NULL);
7861+
7862+
/*
7863+
* We currently reject type tags on non-pointer types,
7864+
* which neither LLVM nor GCC support anyway.
7865+
*/
7866+
if (!t || !btf_type_is_ptr(t))
7867+
return 0;
7868+
7869+
/* We got a pointer, get all associated type tags. */
7870+
for (t = btf_type_by_id(btf, t->type); t && btf_type_is_modifier(t);
7871+
t = btf_type_by_id(btf, t->type)) {
7872+
7873+
/* Skip non-type tag modifiers. */
7874+
if (!btf_type_is_type_tag(t))
7875+
continue;
7876+
7877+
const char *tag = __btf_name_by_offset(btf, t->name_off);
7878+
7879+
if (strcmp(tag, "arena") == 0) {
7880+
*tags |= ARG_TAG_ARENA;
7881+
} else {
7882+
bpf_log(&env->log, "function signature member has unsupported type tag '%s'\n",
7883+
tag);
7884+
return -EOPNOTSUPP;
7885+
}
7886+
}
7887+
7888+
return 0;
7889+
}
7890+
7891+
/* Check whether the type is a valid return type. */
7892+
static int btf_validate_return_type(struct bpf_verifier_env *env, struct btf *btf,
7893+
const struct btf_type *t, int subprog)
7894+
{
7895+
u32 tags = 0;
7896+
int err;
7897+
7898+
err = btf_scan_type_tags(env, btf, t->type, &tags);
7899+
if (err)
7900+
return err;
7901+
7902+
t = btf_type_skip_modifiers(btf, t->type, NULL);
7903+
7904+
/*
7905+
* We allow all subprogs except for the main one to return any kind of arena pointer.
7906+
* General arena variables are not allowed, since it makes no sense to return by value
7907+
* a variable that's on the heap in the first place.
7908+
*/
7909+
if (subprog && (tags & ARG_TAG_ARENA) && btf_type_is_ptr(t))
7910+
return 0;
7911+
7912+
/* We always accept void or scalars. */
7913+
if (btf_type_is_void(t) || btf_type_is_int(t) || btf_is_any_enum(t))
7914+
return 0;
7915+
7916+
return -EOPNOTSUPP;
7917+
}
7918+
78057919
/* Process BTF of a function to produce high-level expectation of function
78067920
* arguments (like ARG_PTR_TO_CTX, or ARG_PTR_TO_MEM, etc). This information
78077921
* is cached in subprog info for reuse.
@@ -7820,6 +7934,7 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
78207934
struct btf *btf = prog->aux->btf;
78217935
const struct btf_param *args;
78227936
const struct btf_type *t, *ref_t, *fn_t;
7937+
int err;
78237938
u32 i, nargs, btf_id;
78247939
const char *tname;
78257940

@@ -7884,61 +7999,30 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
78847999
tname, nargs, MAX_BPF_FUNC_REG_ARGS);
78858000
return -EINVAL;
78868001
}
7887-
/* check that function is void or returns int, exception cb also requires this */
7888-
t = btf_type_by_id(btf, t->type);
7889-
while (btf_type_is_modifier(t))
7890-
t = btf_type_by_id(btf, t->type);
7891-
if (!btf_type_is_void(t) && !btf_type_is_int(t) && !btf_is_any_enum(t)) {
7892-
if (!is_global)
7893-
return -EINVAL;
7894-
bpf_log(log,
7895-
"Global function %s() return value not void or scalar. "
7896-
"Only those are supported.\n",
7897-
tname);
7898-
return -EINVAL;
8002+
8003+
err = btf_validate_return_type(env, btf, t, subprog);
8004+
if (err) {
8005+
if (is_global) {
8006+
bpf_log(log,
8007+
"Global function %s() return value not void or scalar. "
8008+
"Only those are supported.\n",
8009+
tname);
8010+
}
8011+
return err;
78998012
}
79008013

79018014
/* Convert BTF function arguments into verifier types.
79028015
* Only PTR_TO_CTX and SCALAR are supported atm.
79038016
*/
79048017
for (i = 0; i < nargs; i++) {
79058018
u32 tags = 0;
7906-
int id = btf_named_start_id(btf, false) - 1;
7907-
7908-
/* 'arg:<tag>' decl_tag takes precedence over derivation of
7909-
* register type from BTF type itself
7910-
*/
7911-
while ((id = btf_find_next_decl_tag(btf, fn_t, i, "arg:", id)) > 0) {
7912-
const struct btf_type *tag_t = btf_type_by_id(btf, id);
7913-
const char *tag = __btf_name_by_offset(btf, tag_t->name_off) + 4;
7914-
7915-
/* disallow arg tags in static subprogs */
7916-
if (!is_global) {
7917-
bpf_log(log, "arg#%d type tag is not supported in static functions\n", i);
7918-
return -EOPNOTSUPP;
7919-
}
8019+
err = btf_scan_decl_tags(env, btf, fn_t, i, is_global, &tags);
8020+
if (err)
8021+
return err;
79208022

7921-
if (strcmp(tag, "ctx") == 0) {
7922-
tags |= ARG_TAG_CTX;
7923-
} else if (strcmp(tag, "trusted") == 0) {
7924-
tags |= ARG_TAG_TRUSTED;
7925-
} else if (strcmp(tag, "untrusted") == 0) {
7926-
tags |= ARG_TAG_UNTRUSTED;
7927-
} else if (strcmp(tag, "nonnull") == 0) {
7928-
tags |= ARG_TAG_NONNULL;
7929-
} else if (strcmp(tag, "nullable") == 0) {
7930-
tags |= ARG_TAG_NULLABLE;
7931-
} else if (strcmp(tag, "arena") == 0) {
7932-
tags |= ARG_TAG_ARENA;
7933-
} else {
7934-
bpf_log(log, "arg#%d has unsupported set of tags\n", i);
7935-
return -EOPNOTSUPP;
7936-
}
7937-
}
7938-
if (id != -ENOENT) {
7939-
bpf_log(log, "arg#%d type tag fetching failure: %d\n", i, id);
7940-
return id;
7941-
}
8023+
err = btf_scan_type_tags(env, btf, args[i].type, &tags);
8024+
if (err)
8025+
return err;
79428026

79438027
t = btf_type_by_id(btf, args[i].type);
79448028
while (btf_type_is_modifier(t))

kernel/bpf/verifier.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16503,6 +16503,10 @@ static int check_global_subprog_return_code(struct bpf_verifier_env *env)
1650316503
if (err)
1650416504
return err;
1650516505

16506+
/* Pointers to arena are safe to pass between subprograms. */
16507+
if (is_arena_reg(env, BPF_REG_0))
16508+
return 0;
16509+
1650616510
if (is_pointer_value(env, BPF_REG_0)) {
1650716511
verbose(env, "R%d leaks addr as return value\n", BPF_REG_0);
1650816512
return -EACCES;

tools/testing/selftests/bpf/bpf_arena_htab.h

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,16 @@ struct htab {
1414
htab_bucket_t *buckets;
1515
int n_buckets;
1616
};
17-
typedef struct htab __arena htab_t;
1817

19-
static inline htab_bucket_t *__select_bucket(htab_t *htab, __u32 hash)
18+
static inline htab_bucket_t *__select_bucket(struct htab __arena *htab, __u32 hash)
2019
{
2120
htab_bucket_t *b = htab->buckets;
2221

2322
cast_kern(b);
2423
return &b[hash & (htab->n_buckets - 1)];
2524
}
2625

27-
static inline arena_list_head_t *select_bucket(htab_t *htab, __u32 hash)
26+
static inline arena_list_head_t *select_bucket(struct htab __arena *htab, __u32 hash)
2827
{
2928
return &__select_bucket(htab, hash)->head;
3029
}
@@ -53,7 +52,7 @@ static int htab_hash(int key)
5352
return key;
5453
}
5554

56-
__weak int htab_lookup_elem(htab_t *htab __arg_arena, int key)
55+
__weak int htab_lookup_elem(struct htab __arena *htab, int key)
5756
{
5857
hashtab_elem_t *l_old;
5958
arena_list_head_t *head;
@@ -66,7 +65,7 @@ __weak int htab_lookup_elem(htab_t *htab __arg_arena, int key)
6665
return 0;
6766
}
6867

69-
__weak int htab_update_elem(htab_t *htab __arg_arena, int key, int value)
68+
__weak int htab_update_elem(struct htab __arena *htab, int key, int value)
7069
{
7170
hashtab_elem_t *l_new = NULL, *l_old;
7271
arena_list_head_t *head;
@@ -90,7 +89,7 @@ __weak int htab_update_elem(htab_t *htab __arg_arena, int key, int value)
9089
return 0;
9190
}
9291

93-
void htab_init(htab_t *htab)
92+
void htab_init(struct htab __arena *htab)
9493
{
9594
void __arena *buckets = bpf_arena_alloc_pages(&arena, NULL, 2, NUMA_NO_NODE, 0);
9695

tools/testing/selftests/bpf/bpf_arena_strsearch.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
#pragma once
44
#include <bpf_arena_common.h>
55

6-
__noinline int bpf_arena_strlen(const char __arena *s __arg_arena)
6+
__noinline int bpf_arena_strlen(const char __arena *s)
77
{
88
const char __arena *sc;
99

@@ -40,7 +40,7 @@ __noinline int bpf_arena_strlen(const char __arena *s __arg_arena)
4040
*
4141
* An opening bracket without a matching close is matched literally.
4242
*/
43-
__noinline bool glob_match(char const __arena *pat __arg_arena, char const __arena *str __arg_arena)
43+
__noinline bool glob_match(char const __arena *pat, char const __arena *str)
4444
{
4545
/*
4646
* Backtrack to previous * on mismatch and retry starting one

tools/testing/selftests/bpf/libarena/include/bpf_arena_common.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,12 @@
3333
#endif
3434

3535
#if defined(__BPF_FEATURE_ADDR_SPACE_CAST) && !defined(BPF_ARENA_FORCE_ASM)
36-
#define __arena __attribute__((address_space(1)))
36+
#define __arena __attribute__((address_space(1))) __attribute__((btf_type_tag("arena")))
3737
#define __arena_global __attribute__((address_space(1)))
3838
#define cast_kern(ptr) /* nop for bpf prog. emitted by LLVM */
3939
#define cast_user(ptr) /* nop for bpf prog. emitted by LLVM */
4040
#else
41-
#define __arena
41+
#define __arena __attribute__((btf_type_tag("arena")))
4242
#define __arena_global SEC(".addr_space.1")
4343
#define cast_kern(ptr) bpf_addr_space_cast(ptr, 0, 1)
4444
#define cast_user(ptr) bpf_addr_space_cast(ptr, 1, 0)
@@ -54,7 +54,6 @@ void bpf_arena_free_pages(void *map, void __arena *ptr, __u32 page_cnt) __ksym _
5454
#else /* when compiled as user space code */
5555

5656
#define __arena
57-
#define __arg_arena
5857
#define cast_kern(ptr) /* nop for user space */
5958
#define cast_user(ptr) /* nop for user space */
6059
__weak char arena[1];

tools/testing/selftests/bpf/libarena/include/bpf_arena_spin_lock.h

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,6 @@
1616
#define EOPNOTSUPP 95
1717
#define ETIMEDOUT 110
1818

19-
#ifndef __arena
20-
#define __arena __attribute__((address_space(1)))
21-
#endif
22-
2319
extern unsigned long CONFIG_NR_CPUS __kconfig;
2420

2521
/*
@@ -246,7 +242,7 @@ static __always_inline int arena_spin_trylock(arena_spinlock_t __arena *lock)
246242
}
247243

248244
__noinline __weak
249-
int arena_spin_lock_slowpath(arena_spinlock_t __arena __arg_arena *lock, u32 val)
245+
int arena_spin_lock_slowpath(arena_spinlock_t __arena *lock, u32 val)
250246
{
251247
struct arena_mcs_spinlock __arena *prev, *next, *node0, *node;
252248
int ret = -ETIMEDOUT;

tools/testing/selftests/bpf/libarena/include/libarena/asan.h

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,10 @@ extern volatile bool asan_report_once;
2525

2626
#ifdef BPF_ARENA_ASAN
2727

28-
typedef s8 __arena s8a;
29-
3028
static inline
31-
s8a *mem_to_shadow(void __arena __arg_arena *addr)
29+
s8 __arena *mem_to_shadow(void __arena *addr)
3230
{
33-
return (s8a *)(((u32)(u64)addr >> ASAN_SHADOW_SHIFT) +
31+
return (s8 __arena *)(((u32)(u64)addr >> ASAN_SHADOW_SHIFT) +
3432
__asan_shadow_memory_dynamic_address);
3533
}
3634

tools/testing/selftests/bpf/libarena/include/libarena/buddy.h

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,6 @@
22
/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
33
#pragma once
44

5-
struct buddy_chunk;
6-
typedef struct buddy_chunk __arena buddy_chunk_t;
7-
8-
struct buddy_header;
9-
typedef struct buddy_header __arena buddy_header_t;
10-
115
enum buddy_consts {
126
/*
137
* Minimum allocation is 1 << BUDDY_MIN_ALLOC_SHIFT.
@@ -68,25 +62,20 @@ struct buddy_chunk {
6862
u8 allocated[BUDDY_CHUNK_ITEMS / 8];
6963
/* Freelists for O(1) allocation. */
7064
u64 freelists[BUDDY_CHUNK_NUM_ORDERS];
71-
buddy_chunk_t *next;
65+
struct buddy_chunk __arena *next;
7266
};
7367

7468
struct buddy {
75-
buddy_chunk_t *first_chunk; /* Pointer to the chunk linked list. */
69+
struct buddy_chunk __arena *first_chunk; /* Pointer to the chunk linked list. */
7670
arena_spinlock_t lock; /* Allocator lock */
7771
u64 vaddr; /* Allocation into reserved vaddr */
7872
};
7973

80-
typedef struct buddy __arena buddy_t;
81-
8274
#ifdef __BPF__
8375

84-
int buddy_init(buddy_t *buddy);
85-
int buddy_destroy(buddy_t *buddy);
86-
int buddy_free_internal(buddy_t *buddy, u64 free);
87-
#define buddy_free(buddy, ptr) buddy_free_internal((buddy), (u64)(ptr))
88-
u64 buddy_alloc_internal(buddy_t *buddy, size_t size);
89-
#define buddy_alloc(alloc, size) ((void __arena *)buddy_alloc_internal((alloc), (size)))
90-
76+
int buddy_init(struct buddy __arena *buddy);
77+
int buddy_destroy(struct buddy __arena *buddy);
78+
int buddy_free(struct buddy __arena *buddy, void __arena *free);
79+
void __arena *buddy_alloc(struct buddy __arena *buddy, size_t size);
9180

9281
#endif /* __BPF__ */

tools/testing/selftests/bpf/libarena/include/libarena/common.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,7 @@ extern volatile u64 asan_violated;
4848

4949
int arena_fls(__u64 word);
5050

51-
u64 arena_malloc_internal(size_t size);
52-
#define arena_malloc(size) ((void __arena *)arena_malloc_internal((size)))
51+
void __arena *arena_malloc(size_t size);
5352
void arena_free(void __arena *ptr);
5453

5554
/*

0 commit comments

Comments
 (0)