Re: [PATCH bpf-next v13 6/8] bpf: Add syscall common attributes support for map_create

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Calling `close_fd()` on a published file descriptor introduces a critical race condition (File Descriptor Hijacking).
--

commit e059afefe67c83f634eb8a27db293cda839d9654
Author: Leon Hwang <leon.hwang@linux.dev>
bpf: Add syscall common attributes support for map_create

This commit introduces common attributes support and verifier logging to
the map creation process. It refactors the map creation logic to allow
reporting map-creation failure reasons back to userspace.

> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 2fa05ba8f1619..54b43e16848a9 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c

[ ... ]

> @@ -1611,6 +1632,31 @@ static int map_create(union bpf_attr *attr, bpfptr_t uattr)
>  	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);
> +
> +	ret = bpf_log_attr_finalize(&attr_log, log);
> +	if (ret) {
> +		if (err >= 0)
> +			close_fd(err);

Is it safe to call close_fd() here? Since __map_create() uses
bpf_map_new_fd() to allocate and publish the file descriptor via
fd_install(), the file descriptor is already visible to userspace.

Could this introduce a regression where a concurrent thread closes this
file descriptor and opens a new, unrelated file that receives the exact
same fd number? If that happens, it seems this close_fd() might
inadvertently close the unrelated file.

Additionally, since the map is published globally via its ID before fd
installation in bpf_map_alloc_id(), could another thread acquire a
reference to the map before this error path? It looks like the map might
remain alive and accessible even though the syscall returns a failure.

> +		err = ret;
> +	}
> +
> +free:
> +	kfree(log);
> +	return err;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511152817.89191-1-leon.hwang@linux.dev?part=6