From: zhangmingyi <zhangmingyi5@huawei.com>
To: <andrii.nakryiko@gmail.com>
Cc: <andrii@kernel.org>, <ast@kernel.org>, <bpf@vger.kernel.org>,
<daniel@iogearbox.net>, <davem@davemloft.net>,
<edumazet@google.com>, <hsinweih@uci.edu>, <jakub@cloudflare.com>,
<john.fastabend@gmail.com>, <kongweibin2@huawei.com>,
<kuba@kernel.org>, <linux-kernel@vger.kernel.org>,
<liuxin350@huawei.com>, <netdev@vger.kernel.org>,
<pabeni@redhat.com>, <sdf@google.com>,
<syzbot+49f6cef45247ff249498@syzkaller.appspotmail.com>,
<syzkaller-bugs@googlegroups.com>, <wuchangye@huawei.com>,
<xiesongyang@huawei.com>, <yanan@huawei.com>,
<zhangmingyi5@huawei.com>
Subject: Re: [PATCH] libbpf:fix use empty function pointers in ringbuf_poll
Date: Fri, 9 Jun 2023 17:36:25 +0800 [thread overview]
Message-ID: <20230609093625.727490-1-zhangmingyi5@huawei.com> (raw)
In-Reply-To: <CAEf4Bzb2_THiWkqNRnbN5LsOif6+9=GY7LrtEbQf6o24cihhMQ@mail.gmail.com>
On Fri, Jun 9, 2023 at 1:39 AM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> On Thu, Jun 8, 2023 at 9:27 AM Stanislav Fomichev <sdf@google.com> wrote:
> >
> > On Thu, Jun 8, 2023 at 6:00 AM zhangmingyi <zhangmingyi5@huawei.com> wrote:
> > >
> > > On 06/06,Stanislav Fomichev wrote:
> > >
> > > > On 06/05, Xin Liu wrote:
> > > > > From: zhangmingyi <zhangmingyi5@huawei.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.
I agree, we should add a proper doc comment specifying all
arguments and which ones are optional or not.
However, why does the external interface API in libbpf not verify input
parameters or add verification where risky operations may exist?
What's more, i think sample_cb=NULL is not strictly a mistake or
prohibited use, and is meaningless.
next prev parent reply other threads:[~2023-06-09 10:02 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-05 3:34 [PATCH] libbpf:fix use empty function pointers in ringbuf_poll Xin Liu
2023-06-05 17:26 ` Stanislav Fomichev
2023-06-08 12:58 ` zhangmingyi
2023-06-08 16:27 ` Stanislav Fomichev
2023-06-08 17:38 ` Andrii Nakryiko
2023-06-09 9:36 ` zhangmingyi [this message]
2023-06-09 16:55 ` Stanislav Fomichev
2023-06-09 18:23 ` Andrii Nakryiko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230609093625.727490-1-zhangmingyi5@huawei.com \
--to=zhangmingyi5@huawei.com \
--cc=andrii.nakryiko@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=hsinweih@uci.edu \
--cc=jakub@cloudflare.com \
--cc=john.fastabend@gmail.com \
--cc=kongweibin2@huawei.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=liuxin350@huawei.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=sdf@google.com \
--cc=syzbot+49f6cef45247ff249498@syzkaller.appspotmail.com \
--cc=syzkaller-bugs@googlegroups.com \
--cc=wuchangye@huawei.com \
--cc=xiesongyang@huawei.com \
--cc=yanan@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).