On 19/5/26 11:05, Alexei Starovoitov wrote: > On Mon, May 18, 2026 at 7:48 PM Leon Hwang <[email protected]> wrote: >> >> On 18/5/26 23:40, [email protected] wrote: [...] >> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h >> index 20c421b43849..a10ef58bb6ea 100644 >> --- a/include/linux/bpf_verifier.h >> +++ b/include/linux/bpf_verifier.h >> @@ -788,6 +788,7 @@ struct bpf_log_attr { >> u32 level; >> u32 offsetof_true_size; >> bpfptr_t uattr; >> + bool finalized; >> }; > > No. That looks worse. > The suggestion was to do the finalize log _once_ before > allocating FD.
Understand the suggestion. When tried to finalize log once without "finalized" guard, it seemed complicated to modify __map_create() to make sure finalize log once on all code logic paths. > Why are you doing it twice? So, to simplify the change, finalize log the second time with the "finalized" guard to cover all the check-failure paths. After thinking hardly, the below patch is the way to make sure finalize log once before security_bpf_map_create(). It works, but it is not good enough. Thanks, Leon --- >From 80f697c30e830786a57787f370299d4e3335fbbc Mon Sep 17 00:00:00 2001 From: Leon Hwang <[email protected]> Date: Tue, 19 May 2026 15:32:45 +0800 Subject: [PATCH bpf-next v2] bpf: Fix concurrent regression in map_create() Because there is time gap between bpf_map_new_fd() and close_fd(), a concurrent thread is able to close the new fd and opens a new, unrelated file with the exact same fd number. Thereafter, this close_fd() might inadvertently close the unrelated file. To avoid such regression, do finalize log before security_bpf_map_create(). However, to achieve it, move bpf_get_file_flag(), security_bpf_map_create(), bpf_map_alloc_id(), and bpf_map_new_fd() from __map_create() to map_create(). And, rename __map_create() to map_create_alloc() meanwhile. Fixes: 49f9b2b2a18c ("bpf: Add syscall common attributes support for map_create") Signed-off-by: Leon Hwang <[email protected]> --- kernel/bpf/syscall.c | 79 ++++++++++++++++++++++++++------------------ 1 file changed, 46 insertions(+), 33 deletions(-) diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index 83de8fb9b9aa..e815457f5b24 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -1359,7 +1359,8 @@ static int map_check_btf(struct bpf_map *map, struct bpf_token *token, #define BPF_MAP_CREATE_LAST_FIELD excl_prog_hash_size /* called via syscall */ -static int __map_create(union bpf_attr *attr, bpfptr_t uattr, struct bpf_verifier_log *log) +static int map_create_alloc(union bpf_attr *attr, bpfptr_t uattr, struct bpf_verifier_log *log, + struct bpf_map **mapp, struct bpf_token **tokenp) { const struct bpf_map_ops *ops; struct bpf_token *token = NULL; @@ -1367,7 +1368,6 @@ static int __map_create(union bpf_attr *attr, bpfptr_t uattr, struct bpf_verifie u32 map_type = attr->map_type; struct bpf_map *map; bool token_flag; - int f_flags; int err; err = CHECK_ATTR(BPF_MAP_CREATE); @@ -1403,10 +1403,6 @@ static int __map_create(union bpf_attr *attr, bpfptr_t uattr, struct bpf_verifie return -EINVAL; } - f_flags = bpf_get_file_flag(attr->map_flags); - if (f_flags < 0) - return f_flags; - if (numa_node != NUMA_NO_NODE && ((unsigned int)numa_node >= nr_node_ids || !node_online(numa_node))) { @@ -1598,6 +1594,48 @@ static int __map_create(union bpf_attr *attr, bpfptr_t uattr, struct bpf_verifie goto free_map; } + *mapp = map; + *tokenp = token; + return 0; + +free_map: + bpf_map_free(map); +put_token: + bpf_token_put(token); + return err; +} + +static int map_create(union bpf_attr *attr, bpfptr_t uattr, struct bpf_common_attr *attr_common, + bpfptr_t uattr_common, u32 size_common) +{ + struct bpf_token *token = NULL; + struct bpf_verifier_log *log; + struct bpf_log_attr attr_log; + struct bpf_map *map = NULL; + int err, ret; + int f_flags; + + log = bpf_log_attr_create_vlog(&attr_log, attr_common, uattr_common, size_common); + if (IS_ERR(log)) + return PTR_ERR(log); + + err = map_create_alloc(attr, uattr, log, &map, &token); + + /* preserve original error even if log finalization is successful */ + ret = bpf_log_attr_finalize(&attr_log, log); + if (ret) + err = ret; + kfree(log); + + if (err) + goto free_map; + + f_flags = bpf_get_file_flag(attr->map_flags); + if (f_flags < 0) { + err = f_flags; + goto free_map; + } + err = security_bpf_map_create(map, attr, token, uattr.is_kernel); if (err) goto free_map_sec; @@ -1626,37 +1664,12 @@ static int __map_create(union bpf_attr *attr, bpfptr_t uattr, struct bpf_verifie free_map_sec: security_bpf_map_free(map); free_map: - bpf_map_free(map); -put_token: + if (map) + bpf_map_free(map); bpf_token_put(token); return err; } -static int map_create(union bpf_attr *attr, bpfptr_t uattr, struct bpf_common_attr *attr_common, - bpfptr_t uattr_common, u32 size_common) -{ - struct bpf_verifier_log *log; - struct bpf_log_attr attr_log; - int err, ret; - - log = bpf_log_attr_create_vlog(&attr_log, attr_common, uattr_common, size_common); - if (IS_ERR(log)) - return PTR_ERR(log); - - err = __map_create(attr, uattr, log); - - /* preserve original error even if log finalization is successful */ - ret = bpf_log_attr_finalize(&attr_log, log); - if (ret) { - if (err >= 0) - close_fd(err); - err = ret; - } - - kfree(log); - return err; -} - void bpf_map_inc(struct bpf_map *map) { atomic64_inc(&map->refcnt); -- 2.54.0
