Commit 01c8f980 authored by Aleksandr Nogikh's avatar Aleksandr Nogikh Committed by Andrew Morton

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

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: 5ff3b30a ("kcov: collect coverage from interrupts")
Signed-off-by: default avatarAleksandr Nogikh <nogikh@google.com>
Reviewed-by: default avatarDmitry Vyukov <dvyukov@google.com>
Reviewed-by: default avatarAndrey Konovalov <andreyknvl@gmail.com>
Tested-by: default avatarAndrey Konovalov <andreyknvl@gmail.com>
Cc: Alexander Potapenko <glider@google.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Marco Elver <elver@google.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
parent 9094b4a1
...@@ -21,6 +21,8 @@ enum kcov_mode { ...@@ -21,6 +21,8 @@ enum kcov_mode {
KCOV_MODE_TRACE_PC = 2, KCOV_MODE_TRACE_PC = 2,
/* Collecting comparison operands mode. */ /* Collecting comparison operands mode. */
KCOV_MODE_TRACE_CMP = 3, KCOV_MODE_TRACE_CMP = 3,
/* The process owns a KCOV remote reference. */
KCOV_MODE_REMOTE = 4,
}; };
#define KCOV_IN_CTXSW (1 << 30) #define KCOV_IN_CTXSW (1 << 30)
......
...@@ -632,6 +632,7 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd, ...@@ -632,6 +632,7 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd,
return -EINVAL; return -EINVAL;
kcov->mode = mode; kcov->mode = mode;
t->kcov = kcov; t->kcov = kcov;
t->kcov_mode = KCOV_MODE_REMOTE;
kcov->t = t; kcov->t = t;
kcov->remote = true; kcov->remote = true;
kcov->remote_size = remote_arg->area_size; kcov->remote_size = remote_arg->area_size;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment