linux-kernel - Re: [RESEND PATCH bpf-next v4 7/9] bpf: Add common attr support for map

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAADnVQK04pc1ONvqe4JRg+PfSmUK3j1M=vE3A_+GOfYK+8ND9g@mail.gmail.com>
Date: Fri, 9 Jan 2026 13:08:26 -0800
From: Alexei Starovoitov <alexei.starovoitov@...il.com>
To: Leon Hwang <leon.hwang@...ux.dev>
Cc: bot+bpf-ci@...nel.org, bpf <bpf@...r.kernel.org>, 
	Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, 
	Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau <martin.lau@...ux.dev>, Eduard <eddyz87@...il.com>, 
	Song Liu <song@...nel.org>, Yonghong Song <yonghong.song@...ux.dev>, 
	John Fastabend <john.fastabend@...il.com>, KP Singh <kpsingh@...nel.org>, 
	Stanislav Fomichev <sdf@...ichev.me>, Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>, 
	Shuah Khan <shuah@...nel.org>, Christian Brauner <brauner@...nel.org>, Oleg Nesterov <oleg@...hat.com>, 
	sforshee@...nel.org, yuichtsu@...zon.com, aalbersh@...hat.com, 
	Willem de Bruijn <willemb@...gle.com>, Jason Xing <kerneljasonxing@...il.com>, 
	Paul Chaignon <paul.chaignon@...il.com>, Mykyta Yatsenko <yatsenko@...a.com>, 
	Kumar Kartikeya Dwivedi <memxor@...il.com>, Anton Protopopov <a.s.protopopov@...il.com>, 
	Amery Hung <ameryhung@...il.com>, Rong Tao <rongtao@...tc.cn>, 
	LKML <linux-kernel@...r.kernel.org>, Linux API <linux-api@...r.kernel.org>, 
	"open list:KERNEL SELFTEST FRAMEWORK" <linux-kselftest@...r.kernel.org>, kernel-patches-bot@...com, 
	Martin KaFai Lau <martin.lau@...nel.org>, Chris Mason <clm@...a.com>, 
	Ihor Solodrai <ihor.solodrai@...ux.dev>
Subject: Re: [RESEND PATCH bpf-next v4 7/9] bpf: Add common attr support for map_create

On Tue, Jan 6, 2026 at 10:32 PM Leon Hwang <leon.hwang@...ux.dev> wrote:
>
>
>
> On 7/1/26 14:20, Alexei Starovoitov wrote:
> > On Tue, Jan 6, 2026 at 9:57 PM Leon Hwang <leon.hwang@...ux.dev> wrote:
> >>
> >>
> >>
> >> On 7/1/26 01:44, bot+bpf-ci@...nel.org wrote:
> >>>> 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
> >>>
> >>> [ ... ]
> >>>
> >>>> @@ -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