Message-ID: <CAHk-=wh0KoEZXPYMGkfkeVEerSCEF1AiCZSvz9TRrx=Kj74D+Q@mail.gmail.com>
Date:   Mon, 3 May 2021 12:14:45 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Andy Lutomirski <luto@...capital.net>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Stefan Metzmacher <metze@...ba.org>,
        Jens Axboe <axboe@...nel.dk>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        io-uring <io-uring@...r.kernel.org>,
        "the arch/x86 maintainers" <x86@...nel.org>
Subject: Re: [PATCH] io_thread/x86: don't reset 'cs', 'ss', 'ds' and 'es'
 registers for io_threads

On Mon, May 3, 2021 at 9:05 AM Andy Lutomirski <luto@...capital.net> wrote:
>
> Linus, what is the actual effect of allowing gdb to attach these threads?  Can we instead make all the regset ops do:
>
> if (not actually a user thread) return -EINVAL;

I don't think it matters - the end result ends up  being the same, ie
gdb gets confused about whether the (parent) thread is a 32-bit or
64-bit one.

So the basic issue is

 (a) we want the IO threads to look exactly like normal user threads
as far as the kernel is concerned, because we had way too many bugs
due to special cases.

 (b) but that means that they are also visible to user space, and then
gdb has this odd thing where it takes the 64-bit vs 32-bit data for
the whole process from one thread, and picks the worst possible thread
to do it (ie explicitly not even the main thread, so usually the IO
thread!)

That (a) ended up really being critical. The issues with special cases
were just horrendous, both for security issues (ie "make them kernel
threads but carry user credentials" just caused lots of problems) but
also for various just random other state handling issues (signal state
in particular).

So generally, the IO threads are now 100% normal threads - it's
literally just that they never return to user space because they are
always just doing the IO offload on the kernel side.

That part is lovely, but part of the "100% IO threads" really is that
they share the signal struct too, which in turn means that they very
much show up as normal threads. Again, not a problem: they really
_are_ normal threads for all intents and purposes.

But then that (b) issue means that gdb gets confused by them. I
personally think that's just a pure gdb mis-feature, but I also think
that "hey, if we just make the register state look like the main
thread, and unconfuse gdb that way, problem solved".

So I'd actually rather not make these non-special threads any more
special at all. And I strongly suspect that making ptrace() not work
on them will just confuse gdb even more - so it would make them just
unnecessarily special in the kernel, for no actual gain.

Is the right thing to do to fix gdb to not look at irrelevant thread B
when deciding whether thread A is 64-bit or not? Yeah, that seems like
obviously the RightThing(tm) to me.

But at the same time, this is arguably about "regression", although at
the same time it's "gdb doesn't understand new user programs that use
new features, film at 11", so I think that argument is partly bogus
too.

So my personal preference would be:

 - make those threads look even more like user threads, even if that
means giving them pointless user segment data that the threads
themselves will never use

   So I think Stefan's patch is reasonable, if not pretty. Literally
becasue of that "make these threads look even more normal"

 - ALSO fix gdb that is doing obviously garbage stupid things

But I'm obviously not involved in that "ALSO fix gdb" part, and
arguably the kernel hack then makes it more likely that gdb will
continue doing its insane broken thing.

             Linus

Powered by blists - more mailing lists