linux-kernel - Re: [RFC PATCH 1/2] lib/string: Disable instrumentation

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]

Message-ID: <CANpmjNMnU03M0UJiLaHPkRipDuOZht0c9S3d40ZupQVNZLR+RA@mail.gmail.com>
Date:   Tue, 8 Sep 2020 11:39:11 +0200
From:   Marco Elver <elver@...gle.com>
To:     Arvind Sankar <nivedita@...m.mit.edu>
Cc:     "the arch/x86 maintainers" <x86@...nel.org>,
        kasan-dev <kasan-dev@...glegroups.com>,
        Kees Cook <keescook@...omium.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Andrey Konovalov <andreyknvl@...gle.com>,
        Dmitry Vyukov <dvyukov@...gle.com>,
        Alexander Potapenko <glider@...gle.com>
Subject: Re: [RFC PATCH 1/2] lib/string: Disable instrumentation

On Sun, 6 Sep 2020 at 00:23, Arvind Sankar <nivedita@...m.mit.edu> wrote:
>
> String functions can be useful in early boot, but using instrumented
> versions can be problematic: eg on x86, some of the early boot code is
> executing out of an identity mapping rather than the kernel virtual
> addresses. Accessing any global variables at this point will lead to a
> crash.
>
> Tracing and KCOV are already disabled, and CONFIG_AMD_MEM_ENCRYPT will
> additionally disable KASAN and stack protector.
>
> Additionally disable GCOV, UBSAN, KCSAN, STACKLEAK_PLUGIN and branch
> profiling, and make it unconditional to allow safe use of string
> functions.
>
> Signed-off-by: Arvind Sankar <nivedita@...m.mit.edu>
> ---
>  lib/Makefile | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/lib/Makefile b/lib/Makefile
> index a4a4c6864f51..5e421769bbc6 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -8,7 +8,6 @@ ccflags-remove-$(CONFIG_FUNCTION_TRACER) += $(CC_FLAGS_FTRACE)
>  # These files are disabled because they produce lots of non-interesting and/or
>  # flaky coverage that is not a function of syscall inputs. For example,
>  # rbtree can be global and individual rotations don't correlate with inputs.
> -KCOV_INSTRUMENT_string.o := n
>  KCOV_INSTRUMENT_rbtree.o := n
>  KCOV_INSTRUMENT_list_debug.o := n
>  KCOV_INSTRUMENT_debugobjects.o := n
> @@ -20,12 +19,16 @@ KCOV_INSTRUMENT_fault-inject.o := n
>  # them into calls to themselves.
>  CFLAGS_string.o := -ffreestanding
>
> -# Early boot use of cmdline, don't instrument it
> -ifdef CONFIG_AMD_MEM_ENCRYPT
> +# Early boot use of string functions, disable instrumentation
> +GCOV_PROFILE_string.o := n
> +KCOV_INSTRUMENT_string.o := n
>  KASAN_SANITIZE_string.o := n
> +UBSAN_SANITIZE_string.o := n
> +KCSAN_SANITIZE_string.o := n

Ouch.

We have found manifestations of bugs in lib/string.c functions, e.g.:
  https://groups.google.com/forum/#!msg/syzkaller-bugs/atbKWcFqE9s/x7AtoVoBAgAJ
  https://groups.google.com/forum/#!msg/syzkaller-bugs/iGBUm-FDhkM/chl05uEgBAAJ

Is there any way this can be avoided?

If the use of string functions is really necessary, we could introduce
'__'-prefixed variants (maybe only for the ones that are needed?),
a'la

static void __always_inline strfoo_impl(...) { ... }
void strfoo(...) { strfoo_impl(...); }
EXPORT_SYMBOL(strfoo);
noinstr void __strfoo(...) { strfoo_impl(...); }
EXPORT_SYMBOL(__strfoo);
// If __HAVE_ARCH_STRFOO then we can probably just alias __strfoo to strfoo.

But if the whole thing could be avoided entirely would be even better.

Thanks,
-- Marco


>  CFLAGS_string.o += -fno-stack-protector
> -endif
> +CFLAGS_string.o += $(DISABLE_STACKLEAK_PLUGIN)
> +CFLAGS_string.o += -DDISABLE_BRANCH_PROFILING