linux-kernel - Re: [PATCH 0/3] Perf uninitialized value 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] [day] [month] [year] [list]
Message-ID: <CAP-5=fURDxS6DSjc34MNOgqb_UX4YKHT5uHHtzi0X9qVQ8ZqcQ@mail.gmail.com>
Date:   Thu, 22 Aug 2019 14:29:03 -0700
From:   Ian Rogers <irogers@...gle.com>
To:     Arnaldo Carvalho de Melo <arnaldo.melo@...il.com>
Cc:     Numfor Mbiziwo-Tiapo <nums@...gle.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Jiri Olsa <jolsa@...hat.com>,
        Namhyung Kim <namhyung@...nel.org>,
        Song Liu <songliubraving@...com>, mbd@...com,
        LKML <linux-kernel@...r.kernel.org>,
        Stephane Eranian <eranian@...gle.com>
Subject: Re: [PATCH 0/3] Perf uninitialized value fixes

On Wed, Aug 7, 2019 at 1:38 PM Arnaldo Carvalho de Melo
<arnaldo.melo@...il.com> wrote:
>
> Em Wed, Jul 24, 2019 at 04:44:57PM -0700, Numfor Mbiziwo-Tiapo escreveu:
> > These patches are all warnings that the MSAN (Memory Sanitizer) build
> > of perf has caught.
> >
> > To build perf with MSAN enabled run:
> > make -C tools/perf CLANG=1 CC=clang EXTRA_CFLAGS="-fsanitize=memory\
> >  -fsanitize-memory-track-origins"
> >
> > (The -fsanitizer-memory-track-origins makes the bugs clearer but
> > isn't strictly necessary.)
> >
> > (Additionally, llvm might have to be installed and clang might have to
> > be specified as the compiler - export CC=/usr/bin/clang).
> >
> > The patches "Fix util.c use of uninitialized value warning" and "Fix
> > annotate.c use of uninitialized value error" build on top of each other
> > (the changes in Fix util.c use of uninitialized value warning must be
> > made first).
> >
> > When running the commands provided in the repro instructions, MSAN will
> > generate false positive uninitialized memory errors. This is happening
> > because libc is not MSAN-instrumented. Finding a way to build libc with
> > MSAN will get rid of these false positives and allow the real warnings
> > mentioned in the patches to be shown.
>
> So this is because I'm not running a glibc linked with MSAN? Do you have
> any pointer to help building glibc with MSAN? I want to do that inside a
> container so that I can use these sanitizers, thanks,
>
> [root@...co ~]# perf record -o - ls / | perf --no-pager annotate -i -  --stdio
> ==29732==WARNING: MemorySanitizer: use-of-uninitialized-value
> ==29733==WARNING: MemorySanitizer: use-of-uninitialized-value
>     #0 0xcc136d in add_path /home/acme/git/perf/tools/lib/subcmd/exec-cmd.c:130:6
>     #1 0xcc075e in setup_path /home/acme/git/perf/tools/lib/subcmd/exec-cmd.c:146:2
>     #2 0x71298d in main /home/acme/git/perf/tools/perf/perf.c:512:2
>     #0 0xcc136d in add_path /home/acme/git/perf/tools/lib/subcmd/exec-cmd.c:130:6
>     #1 0xcc075e in setup_path /home/acme/git/perf/tools/lib/subcmd/exec-cmd.c:146:2
>     #2 0x71298d in main /home/acme/git/perf/tools/perf/perf.c:512:2
>     #3 0x7f45b9e29f32 in __libc_start_main (/lib64/libc.so.6+0x23f32)
>     #4 0x447dcd in _start (/home/acme/bin/perf+0x447dcd)
>
>   Uninitialized value was created by a heap allocation
>     #3 0x7fd6433cff32 in __libc_start_main (/lib64/libc.so.6+0x23f32)
>     #4 0x447dcd in _start (/home/acme/bin/perf+0x447dcd)
>
>   Uninitialized value was created by a heap allocation
>     #0 0x4507d2 in malloc /home/acme/git/llvm/projects/compiler-rt/lib/msan/msan_interceptors.cc:916:3
>     #1 0x7f45b9e7fc47 in __vasprintf_internal (/lib64/libc.so.6+0x79c47)
>
> SUMMARY: MemorySanitizer: use-of-uninitialized-value /home/acme/git/perf/tools/lib/subcmd/exec-cmd.c:130:6 in add_path
> Exiting
>     #0 0x4507d2 in malloc /home/acme/git/llvm/projects/compiler-rt/lib/msan/msan_interceptors.cc:916:3
>     #1 0x7fd643425c47 in __vasprintf_internal (/lib64/libc.so.6+0x79c47)
>
> SUMMARY: MemorySanitizer: use-of-uninitialized-value /home/acme/git/perf/tools/lib/subcmd/exec-cmd.c:130:6 in add_path
> Exiting
> [root@...co ~]#
>
> > Numfor Mbiziwo-Tiapo (3):
> >   Fix util.c use of uninitialized value warning
> >   Fix annotate.c use of uninitialized value error
> >   Fix sched-messaging.c use of uninitialized value errors
> >
> >  tools/perf/bench/sched-messaging.c |  3 ++-
> >  tools/perf/util/annotate.c         | 15 +++++++++++----
> >  tools/perf/util/header.c           |  2 +-
> >  3 files changed, 14 insertions(+), 6 deletions(-)

Thanks Arnaldo! Debugging the issue it isn't down glibc, there are
interceptors in the sanitizers for asprintf to recognize it as a
source of memory allocation. The problem is the sanitizers don't
support _FORTIFY_SOURCE [1] and this is causing the false positives.
The following patch works to resolve the false-positive issue for me:

-----
--- a/tools/lib/subcmd/Makefile
+++ b/tools/lib/subcmd/Makefile
@@ -20,7 +20,13 @@ MAKEFLAGS += --no-print-directory
LIBFILE = $(OUTPUT)libsubcmd.a

CFLAGS := $(EXTRA_WARNINGS) $(EXTRA_CFLAGS)
-CFLAGS += -ggdb3 -Wall -Wextra -std=gnu99 -U_FORTIFY_SOURCE
-D_FORTIFY_SOURCE=2 -fPIC
+CFLAGS += -ggdb3 -Wall -Wextra -std=gnu99 -fPIC
+
+ifeq ($(DEBUG),0)
+  ifeq ($(feature-fortify-source), 1)
+    CFLAGS += -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2
+  endif
+endif

ifeq ($(CC_NO_CLANG), 0)
  CFLAGS += -O3
-----

Thanks,
Ian

[1] https://github.com/google/sanitizers/wiki/AddressSanitizer#faq

> > --
> > 2.22.0.657.g960e92d24f-goog
>
> --
>
> - Arnaldo