linux-kernel - Re: [PATCH 0/9] x86/entry fixes

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:   Wed, 3 Jun 2020 16:47:54 +0200
From:   Marco Elver <elver@...gle.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        "the arch/x86 maintainers" <x86@...nel.org>,
        "Paul E. McKenney" <paulmck@...nel.org>,
        kasan-dev <kasan-dev@...glegroups.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Will Deacon <will@...nel.org>,
        Dmitry Vyukov <dvyukov@...gle.com>,
        Alexander Potapenko <glider@...gle.com>,
        Andrey Konovalov <andreyknvl@...gle.com>
Subject: Re: [PATCH 0/9] x86/entry fixes

On Wed, 3 Jun 2020 at 15:32, Marco Elver <elver@...gle.com> wrote:
>
> On Wed, 3 Jun 2020 at 14:18, Peter Zijlstra <peterz@...radead.org> wrote:
> >
> > On Wed, Jun 03, 2020 at 02:08:57PM +0200, Marco Elver wrote:
> >
> > > What is the .config you used? I somehow can't reproduce. I've applied
> > > the patches on top of -tip/master.
> >
> > So tip/master, my patches, your patches, this series.
> >
> > $ make CC=/opt/llvm/bin/clang O=defconfig-build/ -j80 -s bzImage
> >
> > is what I used, with the below config.
> >
>
> Thanks, can reproduce now. So far I haven't found any indication that
> there is a missing check in Clang's instrumentation passes somewhere.
> I'm a bit suspicious because both Clang and GCC have this behaviour.
> I'll continue looking.

This is fun: __always_inline functions inlined into
__no_sanitize_undefined *do* get instrumented because apparently UBSan
passes must run before the optimizer (before inlining), contrary to
what [ATM]SAN instrumentation does. Both GCC and Clang do this.

Some options to fix:

1. Add __no_sanitize_undefined to the problematic __always_inline
functions. I don't know if a macro like '#define
__always_inline_noinstr __always_inline __no_sanitize_undefined' is
useful, but it's not an automatic fix either. This option isn't great,
because it doesn't really scale.

2. If you look at the generated code for functions with
__ubsan_handle_*, all the calls are actually guarded by a branch. So
if we know that there is no UBSan violation in the function, AFAIK
we're fine. What are the exact requirements for 'noinstr'? Is it only
"do not call anything I didn't tell you to call?" If that's the case,
and there is no bug in the function ;-), then for UBSan we're fine.
With that in mind, you could whitelist "__ubsan_handle"-prefixed
functions in objtool. Given the __always_inline+noinstr+__ubsan_handle
case is quite rare, it might be reasonable.

We could try to do better, and make __ubsan_handle_* 'noinstr' by
checking if _RET_IP_ is in .noinstr.text and just return. Would that
work? But that would only be useful if there is a UBSan bug. It might
also slow-down regular UBSan, and if we assume that the
__always_inline functions called from noinstr functions that end up
with UBSan instrumentation don't have bugs (big assumption), then not
much is gained either.

Thoughts?

Thanks,
-- Marco