Re: [GIT PULL] orphaned patches for 6.11

From: Linus Torvalds
Date: Wed Jul 24 2024 - 15:34:41 EST


On Wed, 17 Jul 2024 at 03:08, Tetsuo Handa
<penguin-kernel@xxxxxxxxxxxxxxxxxxx> wrote:
>
> These are two of orphaned patches which nobody can take (and as a result
> bugs remain unfixed for years). These patches have been tested using
> linux-next tree via my tomoyo tree since 20240611, and nobody found problems.

Honestly, I've looked at this multiple times, and I think the reason
nobody will take them is that the profiling one in particular seems
horribly overly complex.

Yes, there is most definitely a bug in profiling_store() if multiple
threads call it concurrently, and we try to set up the profiling
buffer multiple times.

Doing that definitely falls under the "Don't do that then!" and is a
root-only operation anyway, but I guess to make syzbot etc happy, it
should be fixed.

But honestly, the minimal fix would seem to be this two-liner:

--- a/kernel/ksysfs.c
+++ b/kernel/ksysfs.c
@@ -92,7 +92,9 @@ static ssize_t profiling_store(struct kobject *kobj,
const char *buf, size_t count)
{
int ret;
+ static DEFINE_MUTEX(prof_store_mutex);

+ guard(mutex)(&prof_store_mutex);
if (prof_on)
return -EEXIST;
/*

which I have admittedly not tested at all, but seems trivial.

And once that "no more multiple concurrent profile initialization" bug
is fixed, everything else is fine. The assignment to "prof_buffer"
will now be the last thing that is done, and when it's done the
profiling should all be good.

All the other problems came from prof_buffer becoming NULL again after
it had already been initialized, because of the "let's re-initialize
it and the first allocation fails" issue.

IOW, I think the above trivial patch is sufficient to fix the problem,
and I don't think this kind of "only syzbot would do this anyway"
issue is worth any more than those two lines.

Have I missed something?

The other patch in there (to just arbitrarily limit max slots to 1k)
looks fine and simple. But I really had to look at the profiling patch
multiple times just to understand what you are fixing, because it
looked so excessively complicated.

Linus