[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CANp29Y6TqZ2T5xKzwW8RJ4o7+4w+mWs2awNebXo1dyaw154Opg@mail.gmail.com>
Date: Wed, 12 Jun 2024 12:11:30 +0200
From: Aleksandr Nogikh <nogikh@...gle.com>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: dvyukov@...gle.com, andreyknvl@...il.com, arnd@...db.de, elver@...gle.com,
glider@...gle.com, syzkaller@...glegroups.com, kasan-dev@...glegroups.com,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] kcov: don't lose track of remote references during softirqs
On Tue, Jun 11, 2024 at 8:51 PM Andrew Morton <akpm@...ux-foundation.org> wrote:
>
> On Tue, 11 Jun 2024 15:32:29 +0200 Aleksandr Nogikh <nogikh@...gle.com> wrote:
>
> > 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.
> >
> > 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().
>
> What are the userspace visible effects of this bug? I *think* it's
> just an efficiency thing, but how significant? In other words, should
> we backport this fix?
>
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.
I think it would be nice to backport the fix to the stable trees.
--
Aleksandr
Powered by blists - more mailing lists