linux-kernel - Re: [PATCH] exit: Put an upper limit on how often we can oops

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-=wjejeg+mymJQYzjc=TeMkGkcOLTgKg4FY4tz4ueYdMsGQ@mail.gmail.com>
Date:   Mon, 7 Nov 2022 12:56:37 -0800
From:   Linus Torvalds <torvalds@...uxfoundation.org>
To:     Jann Horn <jannh@...gle.com>
Cc:     Kees Cook <keescook@...omium.org>, linux-hardening@...r.kernel.org,
        kernel-hardening@...ts.openwall.com,
        Greg KH <gregkh@...uxfoundation.org>,
        Seth Jenkins <sethjenkins@...gle.com>,
        "Eric W . Biederman" <ebiederm@...ssion.com>,
        Andy Lutomirski <luto@...nel.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] exit: Put an upper limit on how often we can oops

On Mon, Nov 7, 2022 at 12:13 PM Jann Horn <jannh@...gle.com> wrote:
>
> I picked 10000 here to also provide safety for the ldsem code on 32-bit
> systems, but you could also argue that the real fix there is to make
> ldsem more robust, and that the limit should be something like 2^31...
>
> An alternative approach would be to always let make_task_dead() take the
> do_task_dead() path and never exit; but that would probably be a more
> disruptive change?

It might be more disruptive, but it might also be a better idea in
some respects: one of the bigger issues we've had with oopses in
inconvenient places is when they then cause even more problems in the
exit path (because the initial oops was horrid).

I'd honestly prefer something higher than 10000, but hey... I would
also prefer something where that legacy 'ldsem' was based on our
current legacy 'struct semaphore' rather than the half-way optimized
'rwsem'. The difference being that 'struct rwsem' tries to be clever
and uses atomic operations, while we long ago decided that anybody who
uses the bad old 'struct semaphore' can just use spinlocks and
non-atomic logic.

It's kind of silly how we try to stuff things into one 'sem->count'
value, when we could just have separate readers and writers counts.

And the only reason we do that is because those kinds of things *do*
matter for contended locks and the rwsem code has it, but I really
think the ldsem code could just always take the spinlock that it
already takes in the slowpath, and just skip any other atomics.

And it shouldn't have a wait_lock thing and two different wait queues
- it should have one wait queue, use that wait queues spinlock *as*
the lock for the semaphore operations, and put readers at the end, and
writers at the beginning as exclusive waiters.

So that ldesc_sem thing is just historical garbage in so many ways.
It's basically a modified copy of an old version of our rwsem, and
hasn't evern been simplified for its intended use, nor has it been
updated to improvements by the actual rwsem code.

Worst of both worlds, in other words.

Oh well. I don't think anybody really cares about the ldsem code,
which is why it is like it is, and probably will remain that way
forever.

I guess 10000 is fine - small enough to test for, big enough that if
somebody really hits it, they only have themselves to blame.

             Linus