BPF List
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: Jiri Olsa <olsajiri@gmail.com>
Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>,
	Juri Lelli <juri.lelli@redhat.com>, bpf <bpf@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Artem Savkov <asavkov@redhat.com>,
	"Jose E. Marchesi" <jose.marchesi@oracle.com>
Subject: Re: NULL pointer deref when running BPF monitor program (6.11.0-rc1)
Date: Tue, 20 Aug 2024 11:05:07 -0400	[thread overview]
Message-ID: <20240820110507.2ba3d541@gandalf.local.home> (raw)
In-Reply-To: <ZsRtOzhicxAhkmoN@krava>

On Tue, 20 Aug 2024 12:17:31 +0200
Jiri Olsa <olsajiri@gmail.com> wrote:

> > Could it be possible that the verifier could add to the exception table for
> > all accesses to tracepoint arguments? Then if there's a NULL pointer
> > dereference, the kernel will not crash but the exception can be sent to the
> > user space process instead? That is, it sends SIGSEV to the task accessing
> > NULL when it shouldn't.  
> 
> hm, but that would mean random process that would happened to trigger
> the tracepoint would segfault, right? I don't think we can do that

Better than a kernel crash, isn't it?  I thought the guarantee of BPF was
not to ever crash the kernel. Crashing user space may be bad, but not
always fatal, and something that can be fixed by fixng the BPF program that
was loaded.

> 
> it seems better to teach verifier which tracepoint arguments can be NULL
> and deny load of the bpf program that would not check such argument properly

These are not mutually exclusive. I think you want both. Adding annotation
is going to be a whack-a-mole game as new tracepoints will always be
created with new possibly NULL parameters and even old tracepoints can add
that too. There's nothing to stop that.

The exception table logic will prevent any missed checks from causing a
kernel crash, and your annotations will keep user space from crashing.

-- Steve

  reply	other threads:[~2024-08-20 15:04 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-05  9:20 NULL pointer deref when running BPF monitor program (6.11.0-rc1) Juri Lelli
2024-08-05 16:49 ` Jiri Olsa
2024-08-05 17:00   ` Alexei Starovoitov
2024-08-06  7:08     ` Juri Lelli
2024-08-06 13:17     ` Jiri Olsa
2024-08-06 13:24     ` Jiri Olsa
2024-08-06 18:44       ` Alexei Starovoitov
2024-08-08 10:46         ` Jiri Olsa
2024-08-08 15:43           ` Alexei Starovoitov
2024-08-15 11:48             ` Jiri Olsa
2024-08-15 12:37               ` Alexei Starovoitov
2024-08-16 14:10                 ` Steven Rostedt
2024-08-16 18:59                   ` Jiri Olsa
2024-08-16 19:30                     ` Steven Rostedt
2024-08-19 11:47                       ` Jiri Olsa
2024-08-19 14:05                         ` Jiri Olsa
2024-08-19 15:37                         ` Steven Rostedt
2024-08-20 10:17                           ` Jiri Olsa
2024-08-20 15:05                             ` Steven Rostedt [this message]
2024-10-02 16:30                               ` Jiri Olsa
2024-10-09 20:41                                 ` Jiri Olsa
2024-10-10  0:33                                   ` Josh Poimboeuf
2024-10-10  0:56                                     ` Steven Rostedt
2024-10-10  0:57                                       ` Steven Rostedt
2024-10-10  3:17                                         ` Josh Poimboeuf
2024-10-10  9:00                                           ` Jiri Olsa
2024-10-10 13:49                                             ` Steven Rostedt

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=20240820110507.2ba3d541@gandalf.local.home \
    --to=rostedt@goodmis.org \
    --cc=alexei.starovoitov@gmail.com \
    --cc=asavkov@redhat.com \
    --cc=bpf@vger.kernel.org \
    --cc=jose.marchesi@oracle.com \
    --cc=juri.lelli@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=olsajiri@gmail.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