linux-stable.git/kernel/kcov.c, branch v6.1.151

kcov: mark in_softirq_really() as __always_inline

2025-01-09T12:30:05+00:00

commit cb0ca08b326aa03f87fe94bb91872ce8d2ef1ed8 upstream.

If gcc decides not to inline in_softirq_really(), objtool warns about a
function call with UACCESS enabled:

kernel/kcov.o: warning: objtool: __sanitizer_cov_trace_pc+0x1e: call to in_softirq_really() with UACCESS enabled
kernel/kcov.o: warning: objtool: check_kcov_mode+0x11: call to in_softirq_really() with UACCESS enabled

Mark this as __always_inline to avoid the problem.

Link: https://lkml.kernel.org/r/20241217071814.2261620-1-arnd@kernel.org
Fixes: 7d4df2dad312 ("kcov: properly check for softirq context")
Signed-off-by: Arnd Bergmann 
Reviewed-by: Marco Elver 
Cc: Aleksandr Nogikh 
Cc: Andrey Konovalov 
Cc: Dmitry Vyukov 
Cc: Josh Poimboeuf 
Cc: Peter Zijlstra 
Cc: 
Signed-off-by: Andrew Morton 
Signed-off-by: Greg Kroah-Hartman

kcov: properly check for softirq context

2024-08-14T11:52:58+00:00

commit 7d4df2dad312f270d62fecb0e5c8b086c6d7dcfc upstream.

When collecting coverage from softirqs, KCOV uses in_serving_softirq() to
check whether the code is running in the softirq context.  Unfortunately,
in_serving_softirq() is > 0 even when the code is running in the hardirq
or NMI context for hardirqs and NMIs that happened during a softirq.

As a result, if a softirq handler contains a remote coverage collection
section and a hardirq with another remote coverage collection section
happens during handling the softirq, KCOV incorrectly detects a nested
softirq coverate collection section and prints a WARNING, as reported by
syzbot.

This issue was exposed by commit a7f3813e589f ("usb: gadget: dummy_hcd:
Switch to hrtimer transfer scheduler"), which switched dummy_hcd to using
hrtimer and made the timer's callback be executed in the hardirq context.

Change the related checks in KCOV to account for this behavior of
in_serving_softirq() and make KCOV ignore remote coverage collection
sections in the hardirq and NMI contexts.

This prevents the WARNING printed by syzbot but does not fix the inability
of KCOV to collect coverage from the __usb_hcd_giveback_urb when dummy_hcd
is in use (caused by a7f3813e589f); a separate patch is required for that.

Link: https://lkml.kernel.org/r/20240729022158.92059-1-andrey.konovalov@linux.dev
Fixes: 5ff3b30ab57d ("kcov: collect coverage from interrupts")
Signed-off-by: Andrey Konovalov 
Reported-by: syzbot+2388cdaeb6b10f0c13ac@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=2388cdaeb6b10f0c13ac
Acked-by: Marco Elver 
Cc: Alan Stern 
Cc: Aleksandr Nogikh 
Cc: Alexander Potapenko 
Cc: Dmitry Vyukov 
Cc: Greg Kroah-Hartman 
Cc: Marcello Sylvester Bauer 
Cc: 
Signed-off-by: Andrew Morton 
Signed-off-by: Greg Kroah-Hartman

kcov: don't lose track of remote references during softirqs

2024-06-27T11:46:22+00:00

commit 01c8f9806bde438ca1c8cbbc439f0a14a6694f6c upstream.

In kcov_remote_start()/kcov_remote_stop(), we swap the previous KCOV
metadata of the current task into a per-CPU variable.  However, the
kcov_mode_enabled(mode) check is not sufficient in the case of remote KCOV
coverage: current->kcov_mode always remains KCOV_MODE_DISABLED for remote
KCOV objects.

If the original task that has invoked the KCOV_REMOTE_ENABLE ioctl happens
to get interrupted and kcov_remote_start() is called, it ultimately leads
to kcov_remote_stop() NOT restoring the original KCOV reference.  So when
the task exits, all registered remote KCOV handles remain active forever.

The most uncomfortable effect (at least for syzkaller) is that the bug
prevents the reuse of the same /sys/kernel/debug/kcov descriptor.  If
we obtain it in the parent process and then e.g.  drop some
capabilities and continuously fork to execute individual programs, at
some point current->kcov of the forked process is lost,
kcov_task_exit() takes no action, and all KCOV_REMOTE_ENABLE ioctls
calls from subsequent forks fail.

And, yes, the efficiency is also affected if we keep on losing remote
kcov objects.
a) kcov_remote_map keeps on growing forever.
b) (If I'm not mistaken), we're also not freeing the memory referenced
by kcov->area.

Fix it by introducing a special kcov_mode that is assigned to the task
that owns a KCOV remote object.  It makes kcov_mode_enabled() return true
and yet does not trigger coverage collection in __sanitizer_cov_trace_pc()
and write_comp_data().

[nogikh@google.com: replace WRITE_ONCE() with an ordinary assignment]
  Link: https://lkml.kernel.org/r/20240614171221.2837584-1-nogikh@google.com
Link: https://lkml.kernel.org/r/20240611133229.527822-1-nogikh@google.com
Fixes: 5ff3b30ab57d ("kcov: collect coverage from interrupts")
Signed-off-by: Aleksandr Nogikh 
Reviewed-by: Dmitry Vyukov 
Reviewed-by: Andrey Konovalov 
Tested-by: Andrey Konovalov 
Cc: Alexander Potapenko 
Cc: Arnd Bergmann 
Cc: Marco Elver 
Cc: 
Signed-off-by: Andrew Morton 
Signed-off-by: Greg Kroah-Hartman

kcov: kmsan: unpoison area->list in kcov_remote_area_put()

2022-10-03T21:03:23+00:00

KMSAN does not instrument kernel/kcov.c for performance reasons (with
CONFIG_KCOV=y virtually every place in the kernel invokes kcov
instrumentation).  Therefore the tool may miss writes from kcov.c that
initialize memory.

When CONFIG_DEBUG_LIST is enabled, list pointers from kernel/kcov.c are
passed to instrumented helpers in lib/list_debug.c, resulting in false
positives.

To work around these reports, we unpoison the contents of area->list after
initializing it.

Link: https://lkml.kernel.org/r/20220915150417.722975-30-glider@google.com
Signed-off-by: Alexander Potapenko 
Cc: Alexander Viro 
Cc: Alexei Starovoitov 
Cc: Andrey Konovalov 
Cc: Andrey Konovalov 
Cc: Andy Lutomirski 
Cc: Arnd Bergmann 
Cc: Borislav Petkov 
Cc: Christoph Hellwig 
Cc: Christoph Lameter 
Cc: David Rientjes 
Cc: Dmitry Vyukov 
Cc: Eric Biggers 
Cc: Eric Biggers 
Cc: Eric Dumazet 
Cc: Greg Kroah-Hartman 
Cc: Herbert Xu 
Cc: Ilya Leoshkevich 
Cc: Ingo Molnar 
Cc: Jens Axboe 
Cc: Joonsoo Kim 
Cc: Kees Cook 
Cc: Marco Elver 
Cc: Mark Rutland 
Cc: Matthew Wilcox 
Cc: Michael S. Tsirkin 
Cc: Pekka Enberg 
Cc: Peter Zijlstra 
Cc: Petr Mladek 
Cc: Stephen Rothwell 
Cc: Steven Rostedt 
Cc: Thomas Gleixner 
Cc: Vasily Gorbik 
Cc: Vegard Nossum 
Cc: Vlastimil Babka 
Signed-off-by: Andrew Morton

kcov: update pos before writing pc in trace function

2022-05-25T20:05:42+00:00

In __sanitizer_cov_trace_pc(), previously we write pc before updating pos.
However, some early interrupt code could bypass check_kcov_mode() check
and invoke __sanitizer_cov_trace_pc().  If such interrupt is raised
between writing pc and updating pos, the pc could be overitten by the
recursive __sanitizer_cov_trace_pc().

As suggested by Dmitry, we cold update pos before writing pc to avoid such
interleaving.

Apply the same change to write_comp_data().

Link: https://lkml.kernel.org/r/20220523053531.1572793-1-liu3101@purdue.edu
Signed-off-by: Congyu Liu 
Reviewed-by: Dmitry Vyukov 
Cc: Andrey Konovalov 
Signed-off-by: Andrew Morton

kcov: don't generate a warning on vm_insert_page()'s failure

2022-04-22T03:01:10+00:00

vm_insert_page()'s failure is not an unexpected condition, so don't do
WARN_ONCE() in such a case.

Instead, print a kernel message and just return an error code.

This flaw has been reported under an OOM condition by sysbot [1].

The message is mainly for the benefit of the test log, in this case the
fuzzer's log so that humans inspecting the log can figure out what was
going on.  KCOV is a testing tool, so I think being a little more chatty
when KCOV unexpectedly is about to fail will save someone debugging
time.

We don't want the WARN, because it's not a kernel bug that syzbot should
report, and failure can happen if the fuzzer tries hard enough (as
above).

Link: https://lkml.kernel.org/r/Ylkr2xrVbhQYwNLf@elver.google.com [1]
Link: https://lkml.kernel.org/r/20220401182512.249282-1-nogikh@google.com
Fixes: b3d7fe86fbd0 ("kcov: properly handle subsequent mmap calls"),
Signed-off-by: Aleksandr Nogikh 
Acked-by: Marco Elver 
Cc: Dmitry Vyukov 
Cc: Andrey Konovalov 
Cc: Alexander Potapenko 
Cc: Taras Madan 
Cc: Sebastian Andrzej Siewior 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds

kcov: properly handle subsequent mmap calls

2022-03-24T02:00:35+00:00

Allocate the kcov buffer during KCOV_MODE_INIT in order to untie mmapping
of a kcov instance and the actual coverage collection process. Modify
kcov_mmap, so that it can be reliably used any number of times once
KCOV_MODE_INIT has succeeded.

These changes to the user-facing interface of the tool only weaken the
preconditions, so all existing user space code should remain compatible
with the new version.

Link: https://lkml.kernel.org/r/20220117153634.150357-3-nogikh@google.com
Signed-off-by: Aleksandr Nogikh 
Reviewed-by: Dmitry Vyukov 
Reviewed-by: Andrey Konovalov 
Cc: Alexander Potapenko 
Cc: Marco Elver 
Cc: Sebastian Andrzej Siewior 
Cc: Taras Madan 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds

kcov: split ioctl handling into locked and unlocked parts

2022-03-24T02:00:35+00:00

Patch series "kcov: improve mmap processing", v3.

Subsequent mmaps of the same kcov descriptor currently do not update the
virtual memory of the task and yet return 0 (success).  This is
counter-intuitive and may lead to unexpected memory access errors.

Also, this unnecessarily limits the functionality of kcov to only the
simplest usage scenarios.  Kcov instances are effectively forever attached
to their first address spaces and it becomes impossible to e.g.  reuse the
same kcov handle in forked child processes without mmapping the memory
first.  This is exactly what we tried to do in syzkaller and inadvertently
came upon this behavior.

This patch series addresses the problem described above.

This patch (of 3):

Currently all ioctls are de facto processed under a spinlock in order to
serialise them.  This, however, prohibits the use of vmalloc and other
memory management functions in the implementations of those ioctls,
unnecessary complicating any further changes to the code.

Let all ioctls first be processed inside the kcov_ioctl() function which
should execute the ones that are not compatible with spinlock and then
pass control to kcov_ioctl_locked() for all other ones.
KCOV_REMOTE_ENABLE is processed both in kcov_ioctl() and
kcov_ioctl_locked() as the steps are easily separable.

Although it is still compatible with a spinlock, move KCOV_INIT_TRACE
handling to kcov_ioctl(), so that the changes from the next commit are
easier to follow.

Link: https://lkml.kernel.org/r/20220117153634.150357-1-nogikh@google.com
Link: https://lkml.kernel.org/r/20220117153634.150357-2-nogikh@google.com
Signed-off-by: Aleksandr Nogikh 
Reviewed-by: Dmitry Vyukov 
Reviewed-by: Andrey Konovalov 
Cc: Marco Elver 
Cc: Alexander Potapenko 
Cc: Taras Madan 
Cc: Sebastian Andrzej Siewior 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds

kcov: replace local_irq_save() with a local_lock_t

2021-11-09T18:02:52+00:00

The kcov code mixes local_irq_save() and spin_lock() in
kcov_remote_{start|end}().  This creates a warning on PREEMPT_RT because
local_irq_save() disables interrupts and spin_lock_t is turned into a
sleeping lock which can not be acquired in a section with disabled
interrupts.

The kcov_remote_lock is used to synchronize the access to the hash-list
kcov_remote_map.  The local_irq_save() block protects access to the
per-CPU data kcov_percpu_data.

There is no compelling reason to change the lock type to raw_spin_lock_t
to make it work with local_irq_save().  Changing it would require to
move memory allocation (in kcov_remote_add()) and deallocation outside
of the locked section.

Adding an unlimited amount of entries to the hashlist will increase the
IRQ-off time during lookup.  It could be argued that this is debug code
and the latency does not matter.  There is however no need to do so and
it would allow to use this facility in an RT enabled build.

Using a local_lock_t instead of local_irq_save() has the befit of adding
a protection scope within the source which makes it obvious what is
protected.  On a !PREEMPT_RT && !LOCKDEP build the local_lock_irqsave()
maps directly to local_irq_save() so there is overhead at runtime.

Replace the local_irq_save() section with a local_lock_t.

Link: https://lkml.kernel.org/r/20210923164741.1859522-6-bigeasy@linutronix.de
Link: https://lore.kernel.org/r/20210830172627.267989-6-bigeasy@linutronix.de
Reported-by: Clark Williams 
Signed-off-by: Sebastian Andrzej Siewior 
Acked-by: Dmitry Vyukov 
Acked-by: Marco Elver 
Tested-by: Marco Elver 
Reviewed-by: Andrey Konovalov 
Cc: Steven Rostedt 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds

kcov: avoid enable+disable interrupts if !in_task()

2021-11-09T18:02:52+00:00

kcov_remote_start() may need to allocate memory in the in_task() case
(otherwise per-CPU memory has been pre-allocated) and therefore requires
enabled interrupts.

The interrupts are enabled before checking if the allocation is required
so if no allocation is required then the interrupts are needlessly enabled
and disabled again.

Enable interrupts only if memory allocation is performed.

Link: https://lkml.kernel.org/r/20210923164741.1859522-5-bigeasy@linutronix.de
Link: https://lore.kernel.org/r/20210830172627.267989-5-bigeasy@linutronix.de
Signed-off-by: Sebastian Andrzej Siewior 
Acked-by: Dmitry Vyukov 
Acked-by: Marco Elver 
Tested-by: Marco Elver 
Reviewed-by: Andrey Konovalov 
Cc: Clark Williams 
Cc: Steven Rostedt 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds