linux-kernel - Re: [PATCH v3 08/10] kcov: add ioctl(KCOV_RESET

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: <CAG_fn=XYS43pefo1EEO6jTTkPHKhB0+hpbh9KGQ5kodAJm3Ncg@mail.gmail.com>
Date: Wed, 6 Aug 2025 11:47:16 +0200
From: Alexander Potapenko <glider@...gle.com>
To: Dmitry Vyukov <dvyukov@...gle.com>
Cc: quic_jiangenj@...cinc.com, linux-kernel@...r.kernel.org, 
	kasan-dev@...glegroups.com, Aleksandr Nogikh <nogikh@...gle.com>, 
	Andrey Konovalov <andreyknvl@...il.com>, Borislav Petkov <bp@...en8.de>, 
	Dave Hansen <dave.hansen@...ux.intel.com>, Ingo Molnar <mingo@...hat.com>, 
	Josh Poimboeuf <jpoimboe@...nel.org>, Marco Elver <elver@...gle.com>, 
	Peter Zijlstra <peterz@...radead.org>, Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH v3 08/10] kcov: add ioctl(KCOV_RESET_TRACE)

On Tue, Jul 29, 2025 at 1:17 PM Dmitry Vyukov <dvyukov@...gle.com> wrote:
>
> On Mon, 28 Jul 2025 at 17:26, Alexander Potapenko <glider@...gle.com> wrote:
> >
> > Provide a mechanism to reset the coverage for the current task
> > without writing directly to the coverage buffer.
> > This is slower, but allows the fuzzers to map the coverage buffer
> > as read-only, making it harder to corrupt.
> >
> > Signed-off-by: Alexander Potapenko <glider@...gle.com>
>
> Reviewed-by: Dmitry Vyukov <dvyukov@...gle.com>
>
>
> >
> > ---
> > v2:
> >  - Update code to match the new description of struct kcov_state
> >
> > Change-Id: I8f9e6c179d93ccbfe0296b14764e88fa837cfffe
> > ---
> >  Documentation/dev-tools/kcov.rst | 26 ++++++++++++++++++++++++++
> >  include/uapi/linux/kcov.h        |  1 +
> >  kernel/kcov.c                    | 15 +++++++++++++++
> >  3 files changed, 42 insertions(+)
> >
> > diff --git a/Documentation/dev-tools/kcov.rst b/Documentation/dev-tools/kcov.rst
> > index 6446887cd1c92..e215c0651e16d 100644
> > --- a/Documentation/dev-tools/kcov.rst
> > +++ b/Documentation/dev-tools/kcov.rst
> > @@ -470,3 +470,29 @@ local tasks spawned by the process and the global task that handles USB bus #1:
> >                 perror("close"), exit(1);
> >         return 0;
> >      }
> > +
> > +
> > +Resetting coverage with an KCOV_RESET_TRACE
> > +-------------------------------------------
> > +
> > +The ``KCOV_RESET_TRACE`` ioctl provides a mechanism to clear collected coverage
> > +data for the current task. It resets the program counter (PC) trace and, if
> > +``KCOV_UNIQUE_ENABLE`` mode is active, also zeroes the associated bitmap.
> > +
> > +The primary use case for this ioctl is to enhance safety during fuzzing.
> > +Normally, a user could map the kcov buffer with ``PROT_READ | PROT_WRITE`` and
> > +reset the trace from the user-space program. However, when fuzzing system calls,
> > +the kernel itself might inadvertently write to this shared buffer, corrupting
> > +the coverage data.
> > +
> > +To prevent this, a fuzzer can map the buffer with ``PROT_READ`` and use
> > +``ioctl(fd, KCOV_RESET_TRACE, 0)`` to safely clear the buffer from the kernel
> > +side before each fuzzing iteration.
> > +
> > +Note that:
> > +
> > +* This ioctl is safer but slower than directly writing to the shared memory
> > +  buffer due to the overhead of a system call.
> > +* ``KCOV_RESET_TRACE`` is itself a system call, and its execution will be traced
> > +  by kcov. Consequently, immediately after the ioctl returns, cover[0] will be
> > +  greater than 0.
> > diff --git a/include/uapi/linux/kcov.h b/include/uapi/linux/kcov.h
> > index e743ee011eeca..8ab77cc3afa76 100644
> > --- a/include/uapi/linux/kcov.h
> > +++ b/include/uapi/linux/kcov.h
> > @@ -23,6 +23,7 @@ struct kcov_remote_arg {
> >  #define KCOV_DISABLE                   _IO('c', 101)
> >  #define KCOV_REMOTE_ENABLE             _IOW('c', 102, struct kcov_remote_arg)
> >  #define KCOV_UNIQUE_ENABLE             _IOW('c', 103, unsigned long)
> > +#define KCOV_RESET_TRACE               _IO('c', 104)
> >
> >  enum {
> >         /*
> > diff --git a/kernel/kcov.c b/kernel/kcov.c
> > index a92c848d17bce..82ed4c6150c54 100644
> > --- a/kernel/kcov.c
> > +++ b/kernel/kcov.c
> > @@ -740,6 +740,21 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd,
> >                 return 0;
> >         case KCOV_UNIQUE_ENABLE:
> >                 return kcov_handle_unique_enable(kcov, arg);
> > +       case KCOV_RESET_TRACE:
> > +               unused = arg;
> > +               if (unused != 0 || current->kcov != kcov)

I think this is too strict, in certain cases it should be possible to
reset the trace not belonging to the current thread, WDYT?
E.g. syzkaller does that for the extra coverage:
https://github.com/google/syzkaller/blob/ffe1dd46b97d508a7b65c279b8108eeaade66cb1/executor/executor.cc#L920