Re: [PATCH bpf-next 2/5] bpf: Fix concurrent regression in map_create()

[Mykyta Yatsenko <mykyta.yatsenko5@gmail.com>]

On 5/18/26 3:54 PM, Leon Hwang wrote:
> 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, drop close_fd() and override err when failed to
> create map and failed to finalize the log.
> 
> In other word, when succeed in creating map but fail to finalize log,
> users will get the map fd instead of the finalization error.
> 
> Fixes: 49f9b2b2a18c ("bpf: Add syscall common attributes support for map_create")
> Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
> ---
>  kernel/bpf/syscall.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 83de8fb9b9aa..322865a88b3a 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -1647,11 +1647,18 @@ static int map_create(union bpf_attr *attr, bpfptr_t uattr, struct bpf_common_at
>  
>  	/* 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);
> +	if (ret && err < 0)
> +		/*
> +		 * Failed to finalize the log.
> +		 * Should not close_fd(err) here. Since the bpf_map_new_fd()

nit: you can't close_fd(err) here, because err < 0? The comment overall appears to
explain why we are making this change right now, rather than why this works this way.

If map_crate() failed with error code and then log finalization failed, do we really
want to override error code? It sounds like map_create error is more important.

> +		 * has published the map fd, if a concurrent thread closes the
> +		 * fd, then opens new, unrelated file that receives the exact
> +		 * same fd number, close_fd(err) might inadvertently close the
> +		 * unrelated file.
> +		 * As a trade-off, override the err only when failed to finalize
> +		 * the log and failed to create map.
> +		 */
>  		err = ret;
> -	}
>  
>  	kfree(log);
>  	return err;