Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20221125122912.54709-1-sunhao.th@gmail.com> <20221128003800.h2bmqcv5dfqmfbcf@MacBook-Pro-5.local>
 <CACkBjsY_Jy9seMfcMMPbYN-YMubcUzABpMm7VFe8wU+X6LKAUQ@mail.gmail.com> <CAADnVQ+G5AuJgo0iRmGOzzr2sS-Ddz6Dt-_99hS+q=VXPpHH7Q@mail.gmail.com>
In-Reply-To: <CAADnVQ+G5AuJgo0iRmGOzzr2sS-Ddz6Dt-_99hS+q=VXPpHH7Q@mail.gmail.com>
From:   Hao Sun <sunhao.th@gmail.com>
Date:   Mon, 28 Nov 2022 10:58:19 +0800
Message-ID: <CACkBjsadebY1RZ0guYeja4bAG-kKWhz30FmOhf5ARhC8Edf1yA@mail.gmail.com>
Subject: Re: [PATCH bpf-next v3 0/3] bpf: Add LDX/STX/ST sanitize in jited BPF progs
To:     Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc:     bpf <bpf@vger.kernel.org>, Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        John Fastabend <john.fastabend@gmail.com>,
        Andrii Nakryiko <andrii@kernel.org>,
        Martin KaFai Lau <martin.lau@linux.dev>,
        Song Liu <song@kernel.org>, Yonghong Song <yhs@fb.com>,
        KP Singh <kpsingh@kernel.org>,
        Stanislav Fomichev <sdf@google.com>,
        Hao Luo <haoluo@google.com>, Jiri Olsa <jolsa@kernel.org>,
        "David S. Miller" <davem@davemloft.net>,
        LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Alexei Starovoitov <alexei.starovoitov@gmail.com> =E4=BA=8E2022=E5=B9=B411=
=E6=9C=8828=E6=97=A5=E5=91=A8=E4=B8=80 10:12=E5=86=99=E9=81=93=EF=BC=9A
>
> On Sun, Nov 27, 2022 at 5:41 PM Hao Sun <sunhao.th@gmail.com> wrote:
> >
> > Alexei Starovoitov <alexei.starovoitov@gmail.com> =E4=BA=8E2022=E5=B9=
=B411=E6=9C=8828=E6=97=A5=E5=91=A8=E4=B8=80 08:38=E5=86=99=E9=81=93=EF=BC=
=9A
> > >
> > > On Fri, Nov 25, 2022 at 08:29:09PM +0800, Hao Sun wrote:
> > > > The verifier sometimes makes mistakes[1][2] that may be exploited t=
o
> > > > achieve arbitrary read/write. Currently, syzbot is continuously tes=
ting
> > > > bpf, and can find memory issues in bpf syscalls, but it can hardly =
find
> > > > mischecking/bugs in the verifier. We need runtime checks like KASAN=
 in
> > > > BPF programs for this. This patch series implements address sanitiz=
e
> > > > in jited BPF progs for testing purpose, so that tools like syzbot c=
an
> > > > find interesting bugs in the verifier automatically by, if possible=
,
> > > > generating and executing BPF programs that bypass the verifier but =
have
> > > > memory issues, then triggering this sanitizing.
> > >
> > > The above paragraph makes it sound that it's currently impossible to
> > > use kasan with BPF. Which is confusing and incorrect statement.
> > > kasan adds all the necessary instrumentation to BPF interpreter alrea=
dy
> > > and syzbot can perform bug discovery.
> > > syzbot runner should just disable JIT and run all progs via interpret=
er.
> > > Adding all this logic to run JITed progs in kasan kernel is
> > > just unnecessary complexity.
> >
> > Sorry for the confusion, I mean JITed BPF prog can't use KASAN currentl=
y,
> > maybe it should be called BPF_JITED_PROG_KASAN.
> >
> > It's actually useful because JIT is used in most real cases for testing=
/fuzzing,
> > syzbot uses WITH_JIT_ALWAYS_ON[1][2].
>
> Just turn it off in syzbot. jit_always_on is a security feature
> because of speculative execution bugs that can exploit
> any in-kernel interpreter (not only bpf interpreter).
>

Will do that, thanks for the information.

> > For those tools, they may need
> > to run hundred times for each generated BPF prog to find interesting bu=
gs in
> > the verifier, JIT makes it much faster.
>
> Unlikely. With all the overhead of saving a bunch of regs,
> restoring them and calling functions instead of direct load/store
> such JITed code is probably running at the same speed as
> interpreter.
> Also syzbot generated progs are tiny.
> Your oob reproducer is tiny too.
> The speed of execution doesn't matter in such cases.
>

Hard to tell which one is faster, since each execution of insn in the
interpreter requires a jmp.
But you're right, did not think about this, I guess randomly generated
progs that can pass the verifier are normally tiny, so the speed indeed
may not be an issue here.

> > Also, bugs in JIT can be
> > missed if they're
> > disabled.
>
> Disagree. Replacing direct load/store with calls
> doesn't improve JIT test coverage.
>
> Also think long term. Beyond kasan there are various *sans
> that instrument code differently. load/store may not be
> the only insns that should be instrumented.
> So hacking JITs either directly or via verifier isn't going
> to scale.

Right, just let those *sans instrument the interpreter is more scalable.

Thanks
Hao