The arena qualifier currently designates its associated type as belonging to address space 1. This property affects code generation, but is not reflected in the BTF information of the function. This lack of information at the BTF level prevents us from returning arena pointers from global subprograms. Subprogs cannot return any data structure more complex than a scalar, so pointers to structs are rejected as a return type. We have no way of marking the return type as a pointer to an arena, which is safe provided the two subprogs have the same arena. Expand the __arena qualifier to also attach a BTF type tag to the type. This lets us determine whether a variable belongs to an arena from its type alone through BTF parsing. Signed-off-by: Emil Tsalapatis --- tools/testing/selftests/bpf/libarena/include/bpf_arena_common.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/bpf/libarena/include/bpf_arena_common.h b/tools/testing/selftests/bpf/libarena/include/bpf_arena_common.h index 16f8ce832004..445be3c4edec 100644 --- a/tools/testing/selftests/bpf/libarena/include/bpf_arena_common.h +++ b/tools/testing/selftests/bpf/libarena/include/bpf_arena_common.h @@ -33,7 +33,7 @@ #endif #if defined(__BPF_FEATURE_ADDR_SPACE_CAST) && !defined(BPF_ARENA_FORCE_ASM) -#define __arena __attribute__((address_space(1))) +#define __arena __attribute__((address_space(1))) __attribute__((btf_type_tag("arena"))) #define __arena_global __attribute__((address_space(1))) #define cast_kern(ptr) /* nop for bpf prog. emitted by LLVM */ #define cast_user(ptr) /* nop for bpf prog. emitted by LLVM */ -- 2.54.0 The BTF parsing logic for function arguments goes through the arguments' decl tags, but does not go into their type tags. Add type tag parsing for function arguments. Signed-off-by: Emil Tsalapatis --- kernel/bpf/btf.c | 120 +++++++++++++++++++++++++++++++++-------------- 1 file changed, 85 insertions(+), 35 deletions(-) diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index 17d4ab0a8206..c6a930aca67e 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -7802,6 +7802,84 @@ enum btf_arg_tag { ARG_TAG_ARENA = BIT_ULL(5), }; +static int btf_scan_decl_tags(struct bpf_verifier_env *env, + const struct btf *btf, + const struct btf_type *fn_t, + u32 arg_idx, bool is_global, u32 *tags) +{ + int id = btf_named_start_id(btf, false) - 1; + + /* + * The 'arg:' decl_tag takes precedence over the derivation + * of the register type from the BTF type itself. + */ + while ((id = btf_find_next_decl_tag(btf, fn_t, arg_idx, "arg:", id)) > 0) { + const struct btf_type *tag_t = btf_type_by_id(btf, id); + const char *tag = __btf_name_by_offset(btf, tag_t->name_off) + 4; + + /* disallow arg tags in static subprogs */ + if (!is_global) { + bpf_log(&env->log, + "arg#%d type tag is not supported in static functions\n", + arg_idx); + return -EOPNOTSUPP; + } + + if (strcmp(tag, "ctx") == 0) { + *tags |= ARG_TAG_CTX; + } else if (strcmp(tag, "trusted") == 0) { + *tags |= ARG_TAG_TRUSTED; + } else if (strcmp(tag, "untrusted") == 0) { + *tags |= ARG_TAG_UNTRUSTED; + } else if (strcmp(tag, "nonnull") == 0) { + *tags |= ARG_TAG_NONNULL; + } else if (strcmp(tag, "nullable") == 0) { + *tags |= ARG_TAG_NULLABLE; + } else if (strcmp(tag, "arena") == 0) { + *tags |= ARG_TAG_ARENA; + } else { + bpf_log(&env->log, "arg#%d has unsupported set of tags\n", arg_idx); + return -EOPNOTSUPP; + } + } + if (id != -ENOENT) { + bpf_log(&env->log, "arg#%d type tag fetching failure: %d\n", arg_idx, id); + return id; + } + + return 0; +} + +static int btf_scan_type_tags(struct bpf_verifier_env *env, + const struct btf *btf, u32 type_id, + u32 *tags) +{ + const struct btf_type *t; + + /* Find the first pointer type in the chain. */ + t = btf_type_skip_modifiers(btf, type_id, NULL); + if (!t || !btf_type_is_ptr(t)) + return 0; + + /* We got a pointer, get all associated type tags. */ + t = btf_type_by_id(btf, t->type); + while (t && btf_type_is_type_tag(t)) { + const char *tag = __btf_name_by_offset(btf, t->name_off); + + if (strcmp(tag, "arena") == 0) { + *tags |= ARG_TAG_ARENA; + } else { + bpf_log(&env->log, "function signature member has unsupported type tag '%s'\n", + tag); + return -EOPNOTSUPP; + } + + t = btf_type_by_id(btf, t->type); + } + + return 0; +} + /* Process BTF of a function to produce high-level expectation of function * arguments (like ARG_PTR_TO_CTX, or ARG_PTR_TO_MEM, etc). This information * is cached in subprog info for reuse. @@ -7820,6 +7898,7 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog) struct btf *btf = prog->aux->btf; const struct btf_param *args; const struct btf_type *t, *ref_t, *fn_t; + int err; u32 i, nargs, btf_id; const char *tname; @@ -7903,42 +7982,13 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog) */ for (i = 0; i < nargs; i++) { u32 tags = 0; - int id = btf_named_start_id(btf, false) - 1; - - /* 'arg:' decl_tag takes precedence over derivation of - * register type from BTF type itself - */ - while ((id = btf_find_next_decl_tag(btf, fn_t, i, "arg:", id)) > 0) { - const struct btf_type *tag_t = btf_type_by_id(btf, id); - const char *tag = __btf_name_by_offset(btf, tag_t->name_off) + 4; - - /* disallow arg tags in static subprogs */ - if (!is_global) { - bpf_log(log, "arg#%d type tag is not supported in static functions\n", i); - return -EOPNOTSUPP; - } + err = btf_scan_decl_tags(env, btf, fn_t, i, is_global, &tags); + if (err) + return err; - if (strcmp(tag, "ctx") == 0) { - tags |= ARG_TAG_CTX; - } else if (strcmp(tag, "trusted") == 0) { - tags |= ARG_TAG_TRUSTED; - } else if (strcmp(tag, "untrusted") == 0) { - tags |= ARG_TAG_UNTRUSTED; - } else if (strcmp(tag, "nonnull") == 0) { - tags |= ARG_TAG_NONNULL; - } else if (strcmp(tag, "nullable") == 0) { - tags |= ARG_TAG_NULLABLE; - } else if (strcmp(tag, "arena") == 0) { - tags |= ARG_TAG_ARENA; - } else { - bpf_log(log, "arg#%d has unsupported set of tags\n", i); - return -EOPNOTSUPP; - } - } - if (id != -ENOENT) { - bpf_log(log, "arg#%d type tag fetching failure: %d\n", i, id); - return id; - } + err = btf_scan_type_tags(env, btf, args[i].type, &tags); + if (err) + return err; t = btf_type_by_id(btf, args[i].type); while (btf_type_is_modifier(t)) -- 2.54.0 As part of moving to annotation-free BPF arena code, move all typedefs in libarena to wrap the pointer of the type instead of the type itself, e.g., moving from typedef struct example __arena example_t; to typedef struct example * __arena example_t; This is to properly propagate to the typedef the new btf_type_tag("arena") added in the __arena annotation. Type tags are only carried by pointer types. We want to use the tag at BTF parsing time to determine whether a function argument or return value is arena-based or not, without having the programmer explicitly add the type tag. This is all possible by using a typedef'ed pointer type for the argument or return value. Signed-off-by: Emil Tsalapatis --- .../bpf/libarena/include/libarena/asan.h | 8 +- .../bpf/libarena/include/libarena/buddy.h | 18 ++--- .../libarena/selftests/test_asan_buddy.bpf.c | 4 +- .../bpf/libarena/selftests/test_asan_common.h | 2 +- .../bpf/libarena/selftests/test_buddy.bpf.c | 2 +- .../selftests/bpf/libarena/src/asan.bpf.c | 38 ++++----- .../selftests/bpf/libarena/src/buddy.bpf.c | 78 +++++++++---------- .../selftests/bpf/libarena/src/common.bpf.c | 4 +- 8 files changed, 77 insertions(+), 77 deletions(-) diff --git a/tools/testing/selftests/bpf/libarena/include/libarena/asan.h b/tools/testing/selftests/bpf/libarena/include/libarena/asan.h index eb9fc69d9eb0..247b0f40707a 100644 --- a/tools/testing/selftests/bpf/libarena/include/libarena/asan.h +++ b/tools/testing/selftests/bpf/libarena/include/libarena/asan.h @@ -25,13 +25,13 @@ extern volatile bool asan_report_once; #ifdef BPF_ARENA_ASAN -typedef s8 __arena s8a; +typedef s8 __arena *s8a; static inline -s8a *mem_to_shadow(void __arena __arg_arena *addr) +s8a mem_to_shadow(void __arena *addr) { - return (s8a *)(((u32)(u64)addr >> ASAN_SHADOW_SHIFT) + - __asan_shadow_memory_dynamic_address); + return (s8a)(((u32)(u64)addr >> ASAN_SHADOW_SHIFT) + + __asan_shadow_memory_dynamic_address); } __weak __noasan diff --git a/tools/testing/selftests/bpf/libarena/include/libarena/buddy.h b/tools/testing/selftests/bpf/libarena/include/libarena/buddy.h index 00e2437128ef..1c69817f1540 100644 --- a/tools/testing/selftests/bpf/libarena/include/libarena/buddy.h +++ b/tools/testing/selftests/bpf/libarena/include/libarena/buddy.h @@ -3,10 +3,10 @@ #pragma once struct buddy_chunk; -typedef struct buddy_chunk __arena buddy_chunk_t; +typedef struct buddy_chunk __arena *buddy_chunk_t; struct buddy_header; -typedef struct buddy_header __arena buddy_header_t; +typedef struct buddy_header __arena *buddy_header_t; enum buddy_consts { /* @@ -68,24 +68,24 @@ struct buddy_chunk { u8 allocated[BUDDY_CHUNK_ITEMS / 8]; /* Freelists for O(1) allocation. */ u64 freelists[BUDDY_CHUNK_NUM_ORDERS]; - buddy_chunk_t *next; + buddy_chunk_t next; }; struct buddy { - buddy_chunk_t *first_chunk; /* Pointer to the chunk linked list. */ + buddy_chunk_t first_chunk; /* Pointer to the chunk linked list. */ arena_spinlock_t lock; /* Allocator lock */ u64 vaddr; /* Allocation into reserved vaddr */ }; -typedef struct buddy __arena buddy_t; +typedef struct buddy __arena *buddy_t; #ifdef __BPF__ -int buddy_init(buddy_t *buddy); -int buddy_destroy(buddy_t *buddy); -int buddy_free_internal(buddy_t *buddy, u64 free); +int buddy_init(buddy_t buddy); +int buddy_destroy(buddy_t buddy); +int buddy_free_internal(buddy_t buddy, u64 free); #define buddy_free(buddy, ptr) buddy_free_internal((buddy), (u64)(ptr)) -u64 buddy_alloc_internal(buddy_t *buddy, size_t size); +u64 buddy_alloc_internal(buddy_t buddy, size_t size); #define buddy_alloc(alloc, size) ((void __arena *)buddy_alloc_internal((alloc), (size))) diff --git a/tools/testing/selftests/bpf/libarena/selftests/test_asan_buddy.bpf.c b/tools/testing/selftests/bpf/libarena/selftests/test_asan_buddy.bpf.c index 71d1885cf2e1..256d62a03ce7 100644 --- a/tools/testing/selftests/bpf/libarena/selftests/test_asan_buddy.bpf.c +++ b/tools/testing/selftests/bpf/libarena/selftests/test_asan_buddy.bpf.c @@ -8,7 +8,7 @@ /* Required for parsing the ASAN call stacks. */ #include "test_progs_compat.h" -extern buddy_t buddy; +extern struct buddy __arena buddy; #ifdef BPF_ARENA_ASAN @@ -54,7 +54,7 @@ static __always_inline int asan_test_buddy_oob_single(size_t alloc_size) * Factored out because asan_validate_addr is complex enough to cause * verification failures if verified with the rest of asan_test_buddy_uaf_single. */ -__weak int asan_test_buddy_byte(u8 __arena __arg_arena *mem, int i, bool freed) +__weak int asan_test_buddy_byte(u8 __arena *mem, int i, bool freed) { int ret; diff --git a/tools/testing/selftests/bpf/libarena/selftests/test_asan_common.h b/tools/testing/selftests/bpf/libarena/selftests/test_asan_common.h index 1d3edc4372ac..b360efb8c566 100644 --- a/tools/testing/selftests/bpf/libarena/selftests/test_asan_common.h +++ b/tools/testing/selftests/bpf/libarena/selftests/test_asan_common.h @@ -9,7 +9,7 @@ static inline void print_asan_map_state(void __arena *addr) { arena_stdout("%s:%d ASAN %p -> (val: %x gran: %x set: [%s])", __func__, __LINE__, addr, - *(s8a *)(addr), ASAN_GRANULE(addr), + *(s8a)(addr), ASAN_GRANULE(addr), asan_shadow_set(addr) ? "yes" : "no"); } diff --git a/tools/testing/selftests/bpf/libarena/selftests/test_buddy.bpf.c b/tools/testing/selftests/bpf/libarena/selftests/test_buddy.bpf.c index 79e6f0baabfe..b45a306816c0 100644 --- a/tools/testing/selftests/bpf/libarena/selftests/test_buddy.bpf.c +++ b/tools/testing/selftests/bpf/libarena/selftests/test_buddy.bpf.c @@ -6,7 +6,7 @@ #include #include -extern buddy_t buddy; +extern struct buddy __arena buddy; struct segarr_entry { u8 __arena *block; diff --git a/tools/testing/selftests/bpf/libarena/src/asan.bpf.c b/tools/testing/selftests/bpf/libarena/src/asan.bpf.c index 64c5b990086c..4d81a0dcdd5f 100644 --- a/tools/testing/selftests/bpf/libarena/src/asan.bpf.c +++ b/tools/testing/selftests/bpf/libarena/src/asan.bpf.c @@ -110,7 +110,7 @@ volatile bool asan_report_once = false; * to exit due to a missing implementation. Provide a simple implementation * just for memset to use it for poisoning/unpoisoning the map. */ -__weak int asan_memset(s8a __arg_arena *dst, s8 val, size_t size) +__weak int asan_memset(s8a dst, s8 val, size_t size) { size_t i; @@ -121,9 +121,9 @@ __weak int asan_memset(s8a __arg_arena *dst, s8 val, size_t size) } /* Validate a 1-byte access, always within a single byte. */ -static __always_inline bool memory_is_poisoned_1(s8a *addr) +static __always_inline bool memory_is_poisoned_1(s8a addr) { - s8 shadow_value = *(s8a *)mem_to_shadow(addr); + s8 shadow_value = *mem_to_shadow(addr); /* Byte is 0, access is valid. */ if (likely(!shadow_value)) @@ -139,7 +139,7 @@ static __always_inline bool memory_is_poisoned_1(s8a *addr) } /* Validate a 2- 4-, 8-byte access, shadow spans up to 2 bytes. */ -static __always_inline bool memory_is_poisoned_2_4_8(s8a *addr, u64 size) +static __always_inline bool memory_is_poisoned_2_4_8(s8a addr, u64 size) { u64 end = (u64)addr + size - 1; @@ -148,17 +148,17 @@ static __always_inline bool memory_is_poisoned_2_4_8(s8a *addr, u64 size) * overflow above ASAN_GRANULE). */ if (likely(ASAN_GRANULE(end) >= size - 1)) - return memory_is_poisoned_1((s8a *)end); + return memory_is_poisoned_1((s8a)end); /* * Otherwise first byte must be fully unpoisoned, and second byte * must be unpoisoned up to the end of the accessed region. */ - return *(s8a *)mem_to_shadow(addr) || memory_is_poisoned_1((s8a *)end); + return *mem_to_shadow(addr) || memory_is_poisoned_1((s8a)end); } -__weak bool asan_shadow_set(void __arena __arg_arena *addr) +__weak bool asan_shadow_set(void __arena *addr) { return memory_is_poisoned_1(addr); } @@ -166,7 +166,7 @@ __weak bool asan_shadow_set(void __arena __arg_arena *addr) static __always_inline u64 first_nonzero_byte(u64 addr, size_t size) { while (size && can_loop) { - if (unlikely(*(s8a *)addr)) + if (unlikely(*(s8a)addr)) return addr; addr += 1; size -= 1; @@ -175,7 +175,7 @@ static __always_inline u64 first_nonzero_byte(u64 addr, size_t size) return SHADOW_ALL_ZEROES; } -static __always_inline bool memory_is_poisoned_n(s8a *addr, u64 size) +static __always_inline bool memory_is_poisoned_n(s8a addr, u64 size) { u64 ret; u64 start; @@ -189,10 +189,10 @@ static __always_inline bool memory_is_poisoned_n(s8a *addr, u64 size) if (likely(ret == SHADOW_ALL_ZEROES)) return false; - return unlikely(ret != end || ASAN_GRANULE(addr + size - 1) >= *(s8a *)end); + return unlikely(ret != end || ASAN_GRANULE(addr + size - 1) >= *(s8a)end); } -__weak int asan_report(s8a __arg_arena *addr, size_t sz, u32 flags) +__weak int asan_report(s8a addr, size_t sz, u32 flags) { u32 reported = __sync_val_compare_and_swap(&asan_reported, false, true); @@ -211,7 +211,7 @@ __weak int asan_report(s8a __arg_arena *addr, size_t sz, u32 flags) return 0; } -static __always_inline bool check_asan_args(s8a *addr, size_t size, +static __always_inline bool check_asan_args(s8a addr, size_t size, bool *result) { bool valid = true; @@ -253,7 +253,7 @@ static __always_inline bool check_asan_args(s8a *addr, size_t size, static __always_inline bool check_region_inline(intptr_t ptr, size_t size, u32 flags) { - s8a *addr = (s8a *)(u64)ptr; + s8a addr = (s8a)(u64)ptr; bool is_poisoned, is_valid; if (check_asan_args(addr, size, &is_valid)) { @@ -305,19 +305,19 @@ static __always_inline bool check_region_inline(intptr_t ptr, size_t size, } \ __hidden void __asan_report_store##size(intptr_t addr) \ { \ - asan_report((s8a *)addr, size, ASAN_WRITE); \ + asan_report((s8a)addr, size, ASAN_WRITE); \ } \ __hidden void __asan_report_store##size##_noabort(intptr_t addr) \ { \ - asan_report((s8a *)addr, size, ASAN_WRITE); \ + asan_report((s8a)addr, size, ASAN_WRITE); \ } \ __hidden void __asan_report_load##size(intptr_t addr) \ { \ - asan_report((s8a *)addr, size, ASAN_READ); \ + asan_report((s8a)addr, size, ASAN_READ); \ } \ __hidden void __asan_report_load##size##_noabort(intptr_t addr) \ { \ - asan_report((s8a *)addr, size, ASAN_READ); \ + asan_report((s8a)addr, size, ASAN_READ); \ } DEFINE_ASAN_LOAD_STORE(1); @@ -385,7 +385,7 @@ void *__asan_memset(void *p, int c, size_t n) */ __hidden __noasan int asan_poison(void __arena *addr, s8 val, size_t size) { - s8a *shadow; + s8a shadow; size_t len; /* @@ -443,7 +443,7 @@ __hidden __noasan int asan_poison(void __arena *addr, s8 val, size_t size) __hidden __noasan int asan_unpoison(void __arena *addr, size_t size) { size_t partial = size & ASAN_GRANULE_MASK; - s8a *shadow; + s8a shadow; size_t len; /* diff --git a/tools/testing/selftests/bpf/libarena/src/buddy.bpf.c b/tools/testing/selftests/bpf/libarena/src/buddy.bpf.c index 865e00803daa..2e565327b748 100644 --- a/tools/testing/selftests/bpf/libarena/src/buddy.bpf.c +++ b/tools/testing/selftests/bpf/libarena/src/buddy.bpf.c @@ -45,12 +45,12 @@ enum { BUDDY_CHUNK_PAGES = BUDDY_CHUNK_BYTES / __PAGE_SIZE }; -static inline int buddy_lock(buddy_t *buddy) +static inline int buddy_lock(buddy_t buddy) { return arena_spin_lock(&buddy->lock); } -static inline void buddy_unlock(buddy_t *buddy) +static inline void buddy_unlock(buddy_t buddy) { arena_spin_unlock(&buddy->lock); } @@ -61,7 +61,7 @@ static inline void buddy_unlock(buddy_t *buddy) * page alloc kfuncs do not support aligning to a boundary (in this * case 1 MiB, see buddy.h on how this is derived). */ -static int buddy_reserve_arena_vaddr(buddy_t *buddy) +static int buddy_reserve_arena_vaddr(buddy_t buddy) { buddy->vaddr = 0; @@ -73,7 +73,7 @@ static int buddy_reserve_arena_vaddr(buddy_t *buddy) /* * Free up any unused address space. Used only during teardown. */ -static void buddy_unreserve_arena_vaddr(buddy_t *buddy) +static void buddy_unreserve_arena_vaddr(buddy_t buddy) { bpf_arena_free_pages( &arena, (void __arena *)(BUDDY_VADDR_OFFSET + buddy->vaddr), @@ -94,7 +94,7 @@ static void buddy_unreserve_arena_vaddr(buddy_t *buddy) * However, bump allocation must still be atomic because this function * is called without the buddy lock from multiple threads concurrently. */ -__weak int buddy_alloc_arena_vaddr(buddy_t __arg_arena *buddy, u64 *vaddrp) +__weak int buddy_alloc_arena_vaddr(buddy_t buddy, u64 *vaddrp) { u64 vaddr, old, new; @@ -134,7 +134,7 @@ static u64 arena_next_pow2(__u64 n) } __weak -int idx_set_allocated(buddy_chunk_t __arg_arena *chunk, u64 idx, bool allocated) +int idx_set_allocated(buddy_chunk_t chunk, u64 idx, bool allocated) { bool already_allocated; @@ -160,7 +160,7 @@ int idx_set_allocated(buddy_chunk_t __arg_arena *chunk, u64 idx, bool allocated) return 0; } -static int idx_is_allocated(buddy_chunk_t *chunk, u64 idx, bool *allocated) +static int idx_is_allocated(buddy_chunk_t chunk, u64 idx, bool *allocated) { if (unlikely(idx >= BUDDY_CHUNK_ITEMS)) { arena_stderr("getting state of invalid idx (%llu, max %d)\n", idx, @@ -173,7 +173,7 @@ static int idx_is_allocated(buddy_chunk_t *chunk, u64 idx, bool *allocated) } __weak -int idx_set_order(buddy_chunk_t __arg_arena *chunk, u64 idx, u8 order) +int idx_set_order(buddy_chunk_t chunk, u64 idx, u8 order) { u8 prev_order; @@ -206,7 +206,7 @@ int idx_set_order(buddy_chunk_t __arg_arena *chunk, u64 idx, u8 order) return 0; } -static u8 idx_get_order(buddy_chunk_t *chunk, u64 idx) +static u8 idx_get_order(buddy_chunk_t chunk, u64 idx) { u8 result; @@ -223,7 +223,7 @@ static u8 idx_get_order(buddy_chunk_t *chunk, u64 idx) return (idx & 0x1) ? (result & 0xf) : (result >> 4); } -static void __arena *idx_to_addr(buddy_chunk_t *chunk, size_t idx) +static void __arena *idx_to_addr(buddy_chunk_t chunk, size_t idx) { u64 address; @@ -246,7 +246,7 @@ static void __arena *idx_to_addr(buddy_chunk_t *chunk, size_t idx) return (void __arena *)address; } -static buddy_header_t *idx_to_header(buddy_chunk_t *chunk, size_t idx) +static buddy_header_t idx_to_header(buddy_chunk_t chunk, size_t idx) { bool allocated; u64 address; @@ -283,13 +283,13 @@ static buddy_header_t *idx_to_header(buddy_chunk_t *chunk, size_t idx) * less probable. */ - return (buddy_header_t *)(address + BUDDY_HEADER_OFF); + return (buddy_header_t)(address + BUDDY_HEADER_OFF); } -static void header_add_freelist(buddy_chunk_t *chunk, buddy_header_t *header, +static void header_add_freelist(buddy_chunk_t chunk, buddy_header_t header, u64 idx, u8 order) { - buddy_header_t *tmp_header; + buddy_header_t tmp_header; idx_set_order(chunk, idx, order); @@ -304,10 +304,10 @@ static void header_add_freelist(buddy_chunk_t *chunk, buddy_header_t *header, chunk->freelists[order] = idx; } -static void header_remove_freelist(buddy_chunk_t *chunk, - buddy_header_t *header, u8 order) +static void header_remove_freelist(buddy_chunk_t chunk, + buddy_header_t header, u8 order) { - buddy_header_t *tmp_header; + buddy_header_t tmp_header; if (header->prev_index != BUDDY_CHUNK_ITEMS) { tmp_header = idx_to_header(chunk, header->prev_index); @@ -356,10 +356,10 @@ static u64 size_to_order(size_t size) } __weak -int add_leftovers_to_freelist(buddy_chunk_t __arg_arena *chunk, u32 cur_idx, +int add_leftovers_to_freelist(buddy_chunk_t chunk, u32 cur_idx, u64 min_order, u64 max_order) { - buddy_header_t *header; + buddy_header_t header; u64 ord; u32 idx; @@ -381,10 +381,10 @@ int add_leftovers_to_freelist(buddy_chunk_t __arg_arena *chunk, u32 cur_idx, return 0; } -static buddy_chunk_t *buddy_chunk_get(buddy_t *buddy) +static buddy_chunk_t buddy_chunk_get(buddy_t buddy) { u64 order, ord, min_order, max_order; - buddy_chunk_t *chunk; + buddy_chunk_t chunk; size_t left; int power2; u64 vaddr; @@ -561,9 +561,9 @@ static buddy_chunk_t *buddy_chunk_get(buddy_t *buddy) return chunk; } -__weak int buddy_init(buddy_t __arg_arena *buddy) +__weak int buddy_init(buddy_t buddy) { - buddy_chunk_t *chunk; + buddy_chunk_t chunk; int ret; if (!asan_ready()) @@ -602,9 +602,9 @@ __weak int buddy_init(buddy_t __arg_arena *buddy) * We do not take a lock because we are freeing arena pages, and nobody should * be using the allocator at that point in the execution. */ -__weak int buddy_destroy(buddy_t __arg_arena *buddy) +__weak int buddy_destroy(buddy_t buddy) { - buddy_chunk_t *chunk, *next; + buddy_chunk_t chunk, next; if (!buddy) return -EINVAL; @@ -631,9 +631,9 @@ __weak int buddy_destroy(buddy_t __arg_arena *buddy) return 0; } -__weak u64 buddy_chunk_alloc(buddy_chunk_t __arg_arena *chunk, int order_req) +__weak u64 buddy_chunk_alloc(buddy_chunk_t chunk, int order_req) { - buddy_header_t *header, *tmp_header, *next_header; + buddy_header_t header, tmp_header, next_header; u32 idx, tmpidx, retidx; u64 address; u64 order = 0; @@ -709,9 +709,9 @@ __weak u64 buddy_chunk_alloc(buddy_chunk_t __arg_arena *chunk, int order_req) } /* Scan the existing chunks for available memory. */ -static u64 buddy_alloc_from_existing_chunks(buddy_t *buddy, int order) +static u64 buddy_alloc_from_existing_chunks(buddy_t buddy, int order) { - buddy_chunk_t *chunk; + buddy_chunk_t chunk; u64 address; for (chunk = buddy->first_chunk; chunk != NULL && can_loop; @@ -728,7 +728,7 @@ static u64 buddy_alloc_from_existing_chunks(buddy_t *buddy, int order) * Try an allocation from a newly allocated chunk. Also * incorporate the chunk into the linked list. */ -static u64 buddy_alloc_from_new_chunk(buddy_t *buddy, buddy_chunk_t *chunk, int order) +static u64 buddy_alloc_from_new_chunk(buddy_t buddy, buddy_chunk_t chunk, int order) { u64 address; @@ -750,9 +750,9 @@ static u64 buddy_alloc_from_new_chunk(buddy_t *buddy, buddy_chunk_t *chunk, int return (u64)address; } __weak -u64 buddy_alloc_internal(buddy_t __arg_arena *buddy, size_t size) +u64 buddy_alloc_internal(buddy_t buddy, size_t size) { - buddy_chunk_t *chunk; + buddy_chunk_t chunk; u64 address = (u64)NULL; int order; @@ -788,20 +788,20 @@ u64 buddy_alloc_internal(buddy_t __arg_arena *buddy, size_t size) * data is smaller than the header, we must poison any * unused bytes that were part of the header. */ - if (size < BUDDY_HEADER_OFF + sizeof(buddy_header_t)) + if (size < BUDDY_HEADER_OFF + sizeof(struct buddy_header)) asan_poison((u8 __arena *)address + BUDDY_HEADER_OFF, - BUDDY_POISONED, sizeof(buddy_header_t)); + BUDDY_POISONED, sizeof(struct buddy_header)); asan_unpoison((u8 __arena *)address, size); return address; } -static __always_inline int buddy_free_unlocked(buddy_t *buddy, u64 addr) +static __always_inline int buddy_free_unlocked(buddy_t buddy, u64 addr) { - buddy_header_t *header, *buddy_header; + buddy_header_t header, buddy_header; u64 idx, buddy_idx, tmp_idx; - buddy_chunk_t *chunk; + buddy_chunk_t chunk; bool allocated; u8 order; int ret; @@ -815,7 +815,7 @@ static __always_inline int buddy_free_unlocked(buddy_t *buddy, u64 addr) } /* Get (chunk, idx) out of the address. */ - chunk = (void __arena *)(addr & ~BUDDY_CHUNK_OFFSET_MASK); + chunk = (buddy_chunk_t)(addr & ~BUDDY_CHUNK_OFFSET_MASK); idx = (addr & BUDDY_CHUNK_OFFSET_MASK) / BUDDY_MIN_ALLOC_BYTES; /* Mark the block as unallocated so we can access the header. */ @@ -878,7 +878,7 @@ static __always_inline int buddy_free_unlocked(buddy_t *buddy, u64 addr) return 0; } -__weak int buddy_free_internal(buddy_t __arg_arena *buddy, u64 addr) +__weak int buddy_free_internal(buddy_t buddy, u64 addr) { int ret; diff --git a/tools/testing/selftests/bpf/libarena/src/common.bpf.c b/tools/testing/selftests/bpf/libarena/src/common.bpf.c index 544bf9e1cb38..96220c86a533 100644 --- a/tools/testing/selftests/bpf/libarena/src/common.bpf.c +++ b/tools/testing/selftests/bpf/libarena/src/common.bpf.c @@ -6,7 +6,7 @@ const volatile u32 zero = 0; -buddy_t buddy; +struct buddy __arena buddy; int arena_fls(__u64 word) { @@ -43,7 +43,7 @@ __weak u64 arena_malloc_internal(size_t size) return buddy_alloc_internal(&buddy, size); } -__weak void arena_free(void __arg_arena __arena *ptr) +__weak void arena_free(void __arena *ptr) { buddy_free_internal(&buddy, (u64)ptr); } -- 2.54.0 BPF subprogs currently only return void or scalar values. However, it is also safe to return arena pointers between subprogs in the same BPF program: Arena pointers are guaranteed to be safe for both programs at any point. Expand the verifier to permit returning an arena pointer to the caller. The main subprog is still not allowed to return an arena pointer because arena pointers are internal to the BPF program, and the return values permitted for each main subprog depend on the program type anyway. Signed-off-by: Emil Tsalapatis --- kernel/bpf/btf.c | 50 +++++++++++++++++++++++++++++++++---------- kernel/bpf/verifier.c | 6 +++++- 2 files changed, 44 insertions(+), 12 deletions(-) diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index c6a930aca67e..7a8101879f84 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -7880,6 +7880,36 @@ static int btf_scan_type_tags(struct bpf_verifier_env *env, return 0; } +/* Check whether the type is a valid return type. */ +static int btf_validate_return_type(struct bpf_verifier_env *env, struct btf *btf, + const struct btf_type *t, int subprog) +{ + u32 tags = 0; + int err; + + err = btf_scan_type_tags(env, btf, t->type, &tags); + if (err) + return err; + t = btf_type_by_id(btf, t->type); + + while (btf_type_is_modifier(t)) + t = btf_type_by_id(btf, t->type); + + /* + * We allow all subprogs except for the main one to return any kind of arena pointer. + * General arena variables are not allowed, since it makes no sense to return by value + * a variable that's on the heap in the first place. + */ + if (subprog && (tags & ARG_TAG_ARENA) && btf_type_is_ptr(t)) + return 0; + + /* We always accept void or scalars. */ + if (btf_type_is_void(t) || btf_type_is_int(t) || btf_is_any_enum(t)) + return 0; + + return -EOPNOTSUPP; +} + /* Process BTF of a function to produce high-level expectation of function * arguments (like ARG_PTR_TO_CTX, or ARG_PTR_TO_MEM, etc). This information * is cached in subprog info for reuse. @@ -7963,17 +7993,15 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog) tname, nargs, MAX_BPF_FUNC_REG_ARGS); return -EINVAL; } - /* check that function is void or returns int, exception cb also requires this */ - t = btf_type_by_id(btf, t->type); - while (btf_type_is_modifier(t)) - t = btf_type_by_id(btf, t->type); - if (!btf_type_is_void(t) && !btf_type_is_int(t) && !btf_is_any_enum(t)) { - if (!is_global) - return -EINVAL; - bpf_log(log, - "Global function %s() return value not void or scalar. " - "Only those are supported.\n", - tname); + + err = btf_validate_return_type(env, btf, t, subprog); + if (err) { + if (is_global) { + bpf_log(log, + "Global function %s() return value not void or scalar. " + "Only those are supported.\n", + tname); + } return -EINVAL; } diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index d9bdc3b32c05..087ddc5119e3 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -9373,7 +9373,7 @@ static int check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn, clear_caller_saved_regs(env, caller->regs); invalidate_outgoing_stack_args(env, cur_func(env)); - /* All non-void global functions return a 64-bit SCALAR_VALUE. */ + /* All non-void global functions return a 64-bit SCALAR_VALUE or PTR_TO_ARENA. */ if (!subprog_returns_void(env, subprog)) { mark_reg_unknown(env, caller->regs, BPF_REG_0); caller->regs[BPF_REG_0].subreg_def = DEF_NOT_SUBREG; @@ -16630,6 +16630,10 @@ static int check_global_subprog_return_code(struct bpf_verifier_env *env) if (err) return err; + /* Pointers to arena are safe to pass between subprograms. */ + if (is_arena_reg(env, BPF_REG_0)) + return 0; + if (is_pointer_value(env, BPF_REG_0)) { verbose(env, "R%d leaks addr as return value\n", BPF_REG_0); return -EACCES; -- 2.54.0 Now that BPF subprogs can return arena programs, remove the preprocessor typecasts between u64 and the appropriate arena type for data structure init functions. Signed-off-by: Emil Tsalapatis --- .../bpf/libarena/include/libarena/buddy.h | 6 ++--- .../bpf/libarena/include/libarena/common.h | 3 +-- .../selftests/bpf/libarena/src/buddy.bpf.c | 25 ++++++++++--------- .../selftests/bpf/libarena/src/common.bpf.c | 6 ++--- 4 files changed, 19 insertions(+), 21 deletions(-) diff --git a/tools/testing/selftests/bpf/libarena/include/libarena/buddy.h b/tools/testing/selftests/bpf/libarena/include/libarena/buddy.h index 1c69817f1540..4f2a19e3d0c4 100644 --- a/tools/testing/selftests/bpf/libarena/include/libarena/buddy.h +++ b/tools/testing/selftests/bpf/libarena/include/libarena/buddy.h @@ -83,10 +83,8 @@ typedef struct buddy __arena *buddy_t; int buddy_init(buddy_t buddy); int buddy_destroy(buddy_t buddy); -int buddy_free_internal(buddy_t buddy, u64 free); -#define buddy_free(buddy, ptr) buddy_free_internal((buddy), (u64)(ptr)) -u64 buddy_alloc_internal(buddy_t buddy, size_t size); -#define buddy_alloc(alloc, size) ((void __arena *)buddy_alloc_internal((alloc), (size))) +int buddy_free(buddy_t buddy, void __arena *ptr); +void __arena *buddy_alloc(buddy_t buddy, size_t size); #endif /* __BPF__ */ diff --git a/tools/testing/selftests/bpf/libarena/include/libarena/common.h b/tools/testing/selftests/bpf/libarena/include/libarena/common.h index ca1a6c1d6477..a3eb1641ac36 100644 --- a/tools/testing/selftests/bpf/libarena/include/libarena/common.h +++ b/tools/testing/selftests/bpf/libarena/include/libarena/common.h @@ -48,8 +48,7 @@ extern volatile u64 asan_violated; int arena_fls(__u64 word); -u64 arena_malloc_internal(size_t size); -#define arena_malloc(size) ((void __arena *)arena_malloc_internal((size))) +void __arena *arena_malloc(size_t size); void arena_free(void __arena *ptr); /* diff --git a/tools/testing/selftests/bpf/libarena/src/buddy.bpf.c b/tools/testing/selftests/bpf/libarena/src/buddy.bpf.c index 2e565327b748..e73145d6dc63 100644 --- a/tools/testing/selftests/bpf/libarena/src/buddy.bpf.c +++ b/tools/testing/selftests/bpf/libarena/src/buddy.bpf.c @@ -750,25 +750,25 @@ static u64 buddy_alloc_from_new_chunk(buddy_t buddy, buddy_chunk_t chunk, int or return (u64)address; } __weak -u64 buddy_alloc_internal(buddy_t buddy, size_t size) +void __arena *buddy_alloc(buddy_t buddy, size_t size) { buddy_chunk_t chunk; - u64 address = (u64)NULL; + u8 __arena *address = NULL; int order; if (!buddy) - return (u64)NULL; + return NULL; order = size_to_order(size); if (order >= BUDDY_CHUNK_NUM_ORDERS || order < 0) { arena_stderr("invalid order %d (sz %lu)\n", order, size); - return (u64)NULL; + return NULL; } if (buddy_lock(buddy)) - return (u64)NULL; + return NULL; - address = buddy_alloc_from_existing_chunks(buddy, order); + address = (u8 __arena *)buddy_alloc_from_existing_chunks(buddy, order); buddy_unlock(buddy); if (address) goto done; @@ -776,12 +776,12 @@ u64 buddy_alloc_internal(buddy_t buddy, size_t size) /* Get a new chunk. */ chunk = buddy_chunk_get(buddy); if (chunk) - address = buddy_alloc_from_new_chunk(buddy, chunk, order); + address = (u8 __arena *)buddy_alloc_from_new_chunk(buddy, chunk, order); done: /* If we failed to allocate memory, return NULL. */ if (!address) - return (u64)NULL; + return NULL; /* * Unpoison exactly the amount of bytes requested. If the @@ -789,10 +789,10 @@ u64 buddy_alloc_internal(buddy_t buddy, size_t size) * unused bytes that were part of the header. */ if (size < BUDDY_HEADER_OFF + sizeof(struct buddy_header)) - asan_poison((u8 __arena *)address + BUDDY_HEADER_OFF, - BUDDY_POISONED, sizeof(struct buddy_header)); + asan_poison(address + BUDDY_HEADER_OFF, BUDDY_POISONED, + sizeof(struct buddy_header)); - asan_unpoison((u8 __arena *)address, size); + asan_unpoison(address, size); return address; } @@ -878,8 +878,9 @@ static __always_inline int buddy_free_unlocked(buddy_t buddy, u64 addr) return 0; } -__weak int buddy_free_internal(buddy_t buddy, u64 addr) +__weak int buddy_free(buddy_t buddy, void __arena *ptr) { + u64 addr = (u64)ptr; int ret; if (!buddy) diff --git a/tools/testing/selftests/bpf/libarena/src/common.bpf.c b/tools/testing/selftests/bpf/libarena/src/common.bpf.c index 96220c86a533..50be57213dfb 100644 --- a/tools/testing/selftests/bpf/libarena/src/common.bpf.c +++ b/tools/testing/selftests/bpf/libarena/src/common.bpf.c @@ -38,14 +38,14 @@ __weak int arena_buddy_reset(void) return buddy_init(&buddy); } -__weak u64 arena_malloc_internal(size_t size) +__weak void __arena *arena_malloc(size_t size) { - return buddy_alloc_internal(&buddy, size); + return buddy_alloc(&buddy, size); } __weak void arena_free(void __arena *ptr) { - buddy_free_internal(&buddy, (u64)ptr); + buddy_free(&buddy, ptr); } -- 2.54.0