Re: [RESEND PATCH bpf-next v4 7/9] bpf: Add common attr support for map_create
From: Alexei Starovoitov <hidden>
Date: 2026-01-09 21:08:39
Also in:
bpf, linux-kselftest, lkml
On Tue, Jan 6, 2026 at 10:32 PM Leon Hwang [off-list ref] wrote:
On 7/1/26 14:20, Alexei Starovoitov wrote:quoted
On Tue, Jan 6, 2026 at 9:57 PM Leon Hwang [off-list ref] wrote:quoted
On 7/1/26 01:44, bot+bpf-ci@kernel.org wrote:quoted
quoted
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index 14fc5738f2b9..e64cc7504731 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c[ ... ]quoted
@@ -6279,7 +6345,11 @@ static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr, unsigned int size, switch (cmd) { case BPF_MAP_CREATE: - err = map_create(&attr, uattr); + common_attrs.log_true_size = 0; + err = map_create(&attr, uattr, &common_attrs); + ret = copy_common_attr_log_true_size(uattr_common, size_common, + &common_attrs.log_true_size); + err = ret ? ret : err;When map_create() succeeds, it returns a file descriptor that is already installed in the caller's fd table via bpf_map_new_fd(). If copy_common_attr_log_true_size() then fails (e.g., user provided a read-only buffer for uattr_common), the syscall returns -EFAULT but the fd remains installed. Could this leak the file descriptor? The user gets an error and has no way to know what fd number was allocated, so they cannot close it.Good catch — you’re right. If 'map_create()' succeeds and 'copy_common_attr_log_true_size()' later fails (e.g. returning -EFAULT), the newly created file descriptor would remain installed and could be leaked. I’ll fix this in the next revision by explicitly closing the fd when ret is non-zero.No. The refactoring was wrong. Don't make the kernel do extra work. Patch 3 introduced a bug and closing fd is not a solution. Such a pattern can be exploited for DoS.You’re right — closing the fd after the fact is not the correct solution, and introducing extra work in the kernel is undesirable. Doing so could also open the door to DoS-style abuse. The correct approach is to copy log_true_size into common_attrs before allocating and installing the new fd, so that a failure in copying cannot leave behind a partially created object.
Why move it at all?
I don't think you should be moving
copy_to_bpfptr_offset(uattr, offsetof(union bpf_attr, log_true_size),
&log_true_size, sizeof(log_true_size))
from where it is in verifier.c