Re: [PATCH bpf-next v14 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:
- [High] Race condition causing unrelated file descriptor closure and resource leakage due to failure after fd_install().
--

commit 37e50113e50fb4d6482363699239289046be6ff2
Author: Leon Hwang <leon.hwang@linux.dev>

bpf: Add syscall common attributes support for map_create

This commit adds common syscall log attributes to map_create(). It creates
a verifier log to report map-creation failure reasons back to userspace,
improving debuggability.

> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 2fa05ba8f1619..6600e126fbfb4 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);
> +
> +	/* 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);

Since __map_create() has already successfully created the map and installed
the file descriptor via fd_install(), the fd is globally visible to the
process at this point.

If bpf_log_attr_finalize() fails (for example, due to an invalid pointer
for log_true_size), we attempt to roll back by calling close_fd(err) and
returning an error.

Could this race with a concurrent thread?

If another thread closes this fd and opens a new file between fd_install()
and this close_fd() call, wouldn't we erroneously close an unrelated file?

Also, if another thread dups the fd before we close it, wouldn't the BPF
map remain alive and accessible to userspace even though the map_create
syscall returned a failure?

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

[ ... ]

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