x86, kcov: ignore stack trace coverage

[PATCH] x86, kcov: ignore stack trace coverage

Posted by andrey.konovalov@linux.dev 1 year, 4 months ago

From: Andrey Konovalov <andreyknvl@gmail.com>

When a multitude of kernel debugging options are enabled, they often
collect and save the current stack trace. The coverage produced by the
related routines is not relevant for the KCOV's intended use case
(guiding the fuzzing process).

Thus, disable instrumentation of the x86 stack trace collection code.

KCOV instrumentaion of the generic kernel/stacktrace.c was already
disabled in commit 43e76af85fa7 ("kcov: ignore fault-inject and
stacktrace"). This patch is an x86-specific addition.

In addition to freeing up the KCOV buffer capacity for holding more
relevant coverage, this patch also speeds up the kernel boot time with
the config from the syzbot USB fuzzing instance by ~25%.

Fixes: 43e76af85fa7 ("kcov: ignore fault-inject and stacktrace")
Signed-off-by: Andrey Konovalov <andreyknvl@gmail.com>

---

I'm not sure whether it makes sense to backport this patch to stable
kernels, but I do think that it makes sense to take it into mainline
as a fix: currently, the USB fuzzing instance is choking on the amount
of coverage produced by KCOV and thus doesn't perform well.

For reference, without this patch, for the following program:

r0 = syz_usb_connect_ath9k(0x3, 0x5a, &(0x7f0000000080)={{0x12, 0x1,
0x200, 0xff, 0xff, 0xff, 0x40, 0xcf3, 0x9271, 0x108, 0x1, 0x2, 0x3, 0x1,
[{{0x9, 0x2, 0x48, 0x1, 0x1, 0x0, 0x80, 0xfa, {{0x9, 0x4, 0x0, 0x0, 0x6,
0xff, 0x0, 0x0, 0x0, "", {{0x9, 0x5, 0x1, 0x2, 0x200, 0x0, 0x0, 0x0, ""},
{0x9, 0x5, 0x82, 0x2, 0x200, 0x0, 0x0, 0x0, ""}, {0x9, 0x5, 0x83, 0x3,
0x40, 0x1, 0x0, 0x0, ""}, {0x9, 0x5, 0x4, 0x3, 0x40, 0x1, 0x0, 0x0, ""},
{0x9, 0x5, 0x5, 0x2, 0x200, 0x0, 0x0, 0x0, ""}, {0x9, 0x5, 0x6, 0x2,
0x200, 0x0, 0x0, 0x0, ""}}}}}}]}}, 0x0)

KCOV produces ~500k coverage entries.

Here are the top ones sorted by the number of occurrences:

  23027 /home/user/src/arch/x86/kernel/unwind_orc.c:99
  17335 /home/user/src/arch/x86/kernel/unwind_orc.c:100
  16460 /home/user/src/arch/x86/include/asm/stacktrace.h:60 (discriminator 3)
  16460 /home/user/src/arch/x86/include/asm/stacktrace.h:60
  16191 /home/user/src/security/tomoyo/domain.c:183 (discriminator 1)
  16128 /home/user/src/security/tomoyo/domain.c:184 (discriminator 8)
  11384 /home/user/src/arch/x86/kernel/unwind_orc.c:109
  11155 /home/user/src/arch/x86/include/asm/stacktrace.h:59
  10997 /home/user/src/arch/x86/kernel/unwind_orc.c:665
  10768 /home/user/src/include/asm-generic/rwonce.h:67
   9994 /home/user/src/arch/x86/kernel/unwind_orc.c:390
   9994 /home/user/src/arch/x86/kernel/unwind_orc.c:389
  ...

With this patch, the number of entries drops to ~140k.

(For reference, here are the top entries with this patch applied:

  16191 /home/user/src/security/tomoyo/domain.c:183 (discriminator 1)
  16128 /home/user/src/security/tomoyo/domain.c:184 (discriminator 8)
   3528 /home/user/src/security/tomoyo/domain.c:173 (discriminator 2)
   3528 /home/user/src/security/tomoyo/domain.c:173
   3528 /home/user/src/security/tomoyo/domain.c:171 (discriminator 5)
   2877 /home/user/src/lib/vsprintf.c:646
   2672 /home/user/src/lib/vsprintf.c:651
   2672 /home/user/src/lib/vsprintf.c:649
   2230 /home/user/src/lib/vsprintf.c:2559
   ...

I'm not sure why tomoyo produces such a large number of entries, but
that will require a separate fix anyway if it's unintended.)
---
 arch/x86/kernel/Makefile | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 20a0dd51700a..241e21723fa5 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -40,6 +40,14 @@ KMSAN_SANITIZE_sev.o					:= n
 KCOV_INSTRUMENT_head$(BITS).o				:= n
 KCOV_INSTRUMENT_sev.o					:= n
 
+# These produce large amounts of uninteresting coverage.
+KCOV_INSTRUMENT_dumpstack.o				:= n
+KCOV_INSTRUMENT_dumpstack_$(BITS).o			:= n
+KCOV_INSTRUMENT_stacktrace.o				:= n
+KCOV_INSTRUMENT_unwind_orc.o				:= n
+KCOV_INSTRUMENT_unwind_frame.o				:= n
+KCOV_INSTRUMENT_unwind_guess.o				:= n
+
 CFLAGS_irq.o := -I $(src)/../include/asm/trace
 
 obj-y			+= head_$(BITS).o
-- 
2.25.1

Re: [PATCH] x86, kcov: ignore stack trace coverage

Posted by Dmitry Vyukov 1 year, 4 months ago

On Mon, 22 Jul 2024 at 22:25, <andrey.konovalov@linux.dev> wrote:
>
> From: Andrey Konovalov <andreyknvl@gmail.com>
>
> When a multitude of kernel debugging options are enabled, they often
> collect and save the current stack trace. The coverage produced by the
> related routines is not relevant for the KCOV's intended use case
> (guiding the fuzzing process).
>
> Thus, disable instrumentation of the x86 stack trace collection code.
>
> KCOV instrumentaion of the generic kernel/stacktrace.c was already
> disabled in commit 43e76af85fa7 ("kcov: ignore fault-inject and
> stacktrace"). This patch is an x86-specific addition.
>
> In addition to freeing up the KCOV buffer capacity for holding more
> relevant coverage, this patch also speeds up the kernel boot time with
> the config from the syzbot USB fuzzing instance by ~25%.
>
> Fixes: 43e76af85fa7 ("kcov: ignore fault-inject and stacktrace")
> Signed-off-by: Andrey Konovalov <andreyknvl@gmail.com>
>
> ---
>
> I'm not sure whether it makes sense to backport this patch to stable
> kernels, but I do think that it makes sense to take it into mainline
> as a fix: currently, the USB fuzzing instance is choking on the amount
> of coverage produced by KCOV and thus doesn't perform well.
>
> For reference, without this patch, for the following program:
>
> r0 = syz_usb_connect_ath9k(0x3, 0x5a, &(0x7f0000000080)={{0x12, 0x1,
> 0x200, 0xff, 0xff, 0xff, 0x40, 0xcf3, 0x9271, 0x108, 0x1, 0x2, 0x3, 0x1,
> [{{0x9, 0x2, 0x48, 0x1, 0x1, 0x0, 0x80, 0xfa, {{0x9, 0x4, 0x0, 0x0, 0x6,
> 0xff, 0x0, 0x0, 0x0, "", {{0x9, 0x5, 0x1, 0x2, 0x200, 0x0, 0x0, 0x0, ""},
> {0x9, 0x5, 0x82, 0x2, 0x200, 0x0, 0x0, 0x0, ""}, {0x9, 0x5, 0x83, 0x3,
> 0x40, 0x1, 0x0, 0x0, ""}, {0x9, 0x5, 0x4, 0x3, 0x40, 0x1, 0x0, 0x0, ""},
> {0x9, 0x5, 0x5, 0x2, 0x200, 0x0, 0x0, 0x0, ""}, {0x9, 0x5, 0x6, 0x2,
> 0x200, 0x0, 0x0, 0x0, ""}}}}}}]}}, 0x0)
>
> KCOV produces ~500k coverage entries.
>
> Here are the top ones sorted by the number of occurrences:
>
>   23027 /home/user/src/arch/x86/kernel/unwind_orc.c:99
>   17335 /home/user/src/arch/x86/kernel/unwind_orc.c:100
>   16460 /home/user/src/arch/x86/include/asm/stacktrace.h:60 (discriminator 3)
>   16460 /home/user/src/arch/x86/include/asm/stacktrace.h:60
>   16191 /home/user/src/security/tomoyo/domain.c:183 (discriminator 1)
>   16128 /home/user/src/security/tomoyo/domain.c:184 (discriminator 8)
>   11384 /home/user/src/arch/x86/kernel/unwind_orc.c:109
>   11155 /home/user/src/arch/x86/include/asm/stacktrace.h:59
>   10997 /home/user/src/arch/x86/kernel/unwind_orc.c:665
>   10768 /home/user/src/include/asm-generic/rwonce.h:67
>    9994 /home/user/src/arch/x86/kernel/unwind_orc.c:390
>    9994 /home/user/src/arch/x86/kernel/unwind_orc.c:389
>   ...
>
> With this patch, the number of entries drops to ~140k.
>
> (For reference, here are the top entries with this patch applied:
>
>   16191 /home/user/src/security/tomoyo/domain.c:183 (discriminator 1)
>   16128 /home/user/src/security/tomoyo/domain.c:184 (discriminator 8)
>    3528 /home/user/src/security/tomoyo/domain.c:173 (discriminator 2)
>    3528 /home/user/src/security/tomoyo/domain.c:173
>    3528 /home/user/src/security/tomoyo/domain.c:171 (discriminator 5)
>    2877 /home/user/src/lib/vsprintf.c:646
>    2672 /home/user/src/lib/vsprintf.c:651
>    2672 /home/user/src/lib/vsprintf.c:649
>    2230 /home/user/src/lib/vsprintf.c:2559
>    ...
>
> I'm not sure why tomoyo produces such a large number of entries, but
> that will require a separate fix anyway if it's unintended.)
> ---
>  arch/x86/kernel/Makefile | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
> index 20a0dd51700a..241e21723fa5 100644
> --- a/arch/x86/kernel/Makefile
> +++ b/arch/x86/kernel/Makefile
> @@ -40,6 +40,14 @@ KMSAN_SANITIZE_sev.o                                 := n
>  KCOV_INSTRUMENT_head$(BITS).o                          := n
>  KCOV_INSTRUMENT_sev.o                                  := n
>
> +# These produce large amounts of uninteresting coverage.
> +KCOV_INSTRUMENT_dumpstack.o                            := n
> +KCOV_INSTRUMENT_dumpstack_$(BITS).o                    := n
> +KCOV_INSTRUMENT_stacktrace.o                           := n
> +KCOV_INSTRUMENT_unwind_orc.o                           := n
> +KCOV_INSTRUMENT_unwind_frame.o                         := n
> +KCOV_INSTRUMENT_unwind_guess.o                         := n

I've sent something similar recently, I think it should be in tip/x86 queue now:
https://lore.kernel.org/all/eaf54b8634970b73552dcd38bf9be6ef55238c10.1718092070.git.dvyukov@google.com/



>  CFLAGS_irq.o := -I $(src)/../include/asm/trace
>
>  obj-y                  += head_$(BITS).o
> --
> 2.25.1
>

Re: [PATCH] x86, kcov: ignore stack trace coverage

Posted by Andrey Konovalov 1 year, 4 months ago

On Mon, Jul 22, 2024 at 10:36 PM Dmitry Vyukov <dvyukov@google.com> wrote:
>
> > +# These produce large amounts of uninteresting coverage.
> > +KCOV_INSTRUMENT_dumpstack.o                            := n
> > +KCOV_INSTRUMENT_dumpstack_$(BITS).o                    := n
> > +KCOV_INSTRUMENT_stacktrace.o                           := n
> > +KCOV_INSTRUMENT_unwind_orc.o                           := n
> > +KCOV_INSTRUMENT_unwind_frame.o                         := n
> > +KCOV_INSTRUMENT_unwind_guess.o                         := n

Ah, I even reviewed it, completely forgot :(

That's great then, thank you!

This patch can be ignored.