linux-kernel - Re: [PATCH] io_thread/x86: don't reset 'cs', 'ss', 'ds' and 'es' registers for io

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: <CALCETrU+f346HXbQAVZ9+hK9SxOy0O_37erBKMis+LGXtgDexw@mail.gmail.com>
Date:   Mon, 3 May 2021 13:21:17 -0700
From:   Andy Lutomirski <luto@...nel.org>
To:     Andy Lutomirski <luto@...nel.org>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        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 1:15 PM Andy Lutomirski <luto@...nel.org> wrote:
>
> On Mon, May 3, 2021 at 12:15 PM Linus Torvalds
> <torvalds@...ux-foundation.org> wrote:
> > 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.
>
> I'm a bit confused, though.  All the ptrace register access (AFAICS)
> goes through ptrace_check_attach(), which should wait until the tracee
> is stopped.  Does the io_uring thread now stop in response to ptrace
> stop requests?
>
> >
> > 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.
> >
>
> Fair enough.  But I would really, really rather that gdb starts fixing
> its amazingly broken assumptions about bitness.
>
> > 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"
>
> I think it's reasonable except for the bit about copying the segment
> regs.  Can we hardcode __USER_CS, etc, and, when gdb crashes or
> otherwise malfunctions for compat programs, we can say that gdb needs
> to stop sucking.  In general, I think that piling a bitness hack in
> here is a mess, and we're going to have to carry it forward forever
> once we do it.

Actually... if we have your permission, I'd rather do the -EINVAL
thing.  Arguably, if gdb breaks cleanly, that's a win.  This only
affects programs using io_uring, it avoids a kludge, and hopefully it
will encourage gdb to fix their bug.  May we do that instead?

--Andy