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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CACT4Y+bmmYuCbV6g9yk8aFZdzhGhct3K78ii6voHR4KAa6oE0g@mail.gmail.com>
Date: Wed, 6 Aug 2025 11:59:35 +0200
From: Dmitry Vyukov <dvyukov@...gle.com>
To: Alexander Potapenko <glider@...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 Wed, 6 Aug 2025 at 11:47, Alexander Potapenko <glider@...gle.com> wrote:
>
> 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

Yes, remote should be allowed here. There is some mutex that protects
remote trace buffer.