netdev - Re: [PATCH] libbpf:fix use empty function pointers in ringbuf

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAEf4Bzb2_THiWkqNRnbN5LsOif6+9=GY7LrtEbQf6o24cihhMQ@mail.gmail.com>
Date: Thu, 8 Jun 2023 10:38:32 -0700
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Stanislav Fomichev <sdf@...gle.com>
Cc: zhangmingyi <zhangmingyi5@...wei.com>, andrii@...nel.org, ast@...nel.org, 
	bpf@...r.kernel.org, daniel@...earbox.net, davem@...emloft.net, 
	edumazet@...gle.com, hsinweih@....edu, jakub@...udflare.com, 
	john.fastabend@...il.com, kongweibin2@...wei.com, kuba@...nel.org, 
	linux-kernel@...r.kernel.org, liuxin350@...wei.com, netdev@...r.kernel.org, 
	pabeni@...hat.com, syzbot+49f6cef45247ff249498@...kaller.appspotmail.com, 
	syzkaller-bugs@...glegroups.com, wuchangye@...wei.com, xiesongyang@...wei.com, 
	yanan@...wei.com
Subject: Re: [PATCH] libbpf:fix use empty function pointers in ringbuf_poll

On Thu, Jun 8, 2023 at 9:27 AM Stanislav Fomichev <sdf@...gle.com> wrote:
>
> On Thu, Jun 8, 2023 at 6:00 AM zhangmingyi <zhangmingyi5@...wei.com> wrote:
> >
> > On 06/06,Stanislav Fomichev wrote:
> >
> > > On 06/05, Xin Liu wrote:
> > > > From: zhangmingyi <zhangmingyi5@...wei.com>
> > >
> > > > The sample_cb of the ring_buffer__new interface can transfer NULL. However,
> > > > the system does not check whether sample_cb is NULL during
> > > > ring_buffer__poll, null pointer is used.
> >
> > > What is the point of calling ring_buffer__new with sample_cb == NULL?
> >
> > Yes, as you said, passing sample_cb in ring_buffer__new to NULL doesn't
> > make sense, and few people use it that way, but that doesn't prevent this
> > from being a allowed and supported scenario. And when ring_buffer__poll is
> > called, it leads to a segmentation fault (core dump), which I think needs
> > to be fixed to ensure the security quality of libbpf.
>
> I dunno. I'd argue that passing a NULL to ring_buffer__new is an API
> misuse. Maybe ring_buffer__new should return -EINVAL instead when
> passed NULL sample_cb? Although, we don't usually have those checks
> for the majority of the arguments in libbpf...

Right. I'd say we should add a proper doc comment specifying all
arguments and which ones are optional or not. And make it explicit
that callback is not optional. If we start checking every possible
pointer for NULL, libbpf will be littered with NULL checks, I'm not
sure that's good.