netdev - Re: [PATCH v3 bpf-next] bpf: make verifier log more relevant by default

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]

Date:   Thu, 5 Nov 2020 14:41:12 -0800
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     Jiri Benc <jbenc@...hat.com>, Andrii Nakryiko <andriin@...com>,
        bpf <bpf@...r.kernel.org>, Networking <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Kernel Team <kernel-team@...com>
Subject: Re: [PATCH v3 bpf-next] bpf: make verifier log more relevant by default

On Thu, Nov 5, 2020 at 1:53 PM Jakub Kicinski <kuba@...nel.org> wrote:
>
> On Thu, 5 Nov 2020 13:22:12 -0800 Andrii Nakryiko wrote:
> > On Thu, Nov 5, 2020 at 8:02 AM Jiri Benc <jbenc@...hat.com> wrote:
> > > On Thu, 23 Apr 2020 12:58:50 -0700, Andrii Nakryiko wrote:
> > > > To make BPF verifier verbose log more releavant and easier to use to debug
> > > > verification failures, "pop" parts of log that were successfully verified.
> > > > This has effect of leaving only verifier logs that correspond to code branches
> > > > that lead to verification failure, which in practice should result in much
> > > > shorter and more relevant verifier log dumps. This behavior is made the
> > > > default behavior and can be overriden to do exhaustive logging by specifying
> > > > BPF_LOG_LEVEL2 log level.
> > >
> > > This patch broke the test_offload.py selftest:
> > >
> > > [...]
> > > Test TC offloads work...
> > > FAIL: Missing or incorrect message from netdevsim in verifier log
> > > [...]
> > >
> > > The selftest expects to receive "[netdevsim] Hello from netdevsim!" in
> > > the log (coming from nsim_bpf_verify_insn) but that part of the log is
> > > cleared by bpf_vlog_reset added by this patch.
> >
> > Should we just drop check_verifier_log() checks?
>
> Drivers only print error messages when something goes wrong, so the
> messages are high priority. IIUC this change was just supposed to
> decrease verbosity, right?

Seems like check_verifier_log() in test_offline.py is only called for
successful cases. This patch truncates parts of the verifier log that
correspond to successfully validated code paths, so that in case if
verification fails, only relevant parts are left. So for completely
successful verification the log will be almost empty, with only final
stats available.