linux-kernel - Re: [GIT PULL] orphaned patches for 6.11

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHk-=whAfNJKeP1WhdP9y0itF_AkgQJMyz8B9TCfAWWQRhDRPw@mail.gmail.com>
Date: Wed, 24 Jul 2024 12:34:10 -0700
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>
Cc: LKML <linux-kernel@...r.kernel.org>
Subject: Re: [GIT PULL] orphaned patches for 6.11

On Wed, 17 Jul 2024 at 03:08, Tetsuo Handa
<penguin-kernel@...ove.sakura.ne.jp> 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