> On May 3, 2021, at 7:00 AM, Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
>
> Stefan,
>
> On Sun, Apr 11 2021 at 17:27, Stefan Metzmacher wrote:
>
> Can you please CC x86 people on patches which are x86 related?
>
>> This allows gdb attach to userspace processes using io-uring,
>> which means that they have io_threads (PF_IO_WORKER), which appear
>> just like normal as userspace threads.
>
> That's not a changelog, really. Please describe what the problem is and
> why the chosen solution is correct.
>
>> See the code comment for more details.
>
> The changelog should be self contained.
>
>> Fixes: 4727dc20e04 ("arch: setup PF_IO_WORKER threads like PF_KTHREAD")
>> Signed-off-by: Stefan Metzmacher <metze@xxxxxxxxx>
>> cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
>> cc: Jens Axboe <axboe@xxxxxxxxx>
>> cc: linux-kernel@xxxxxxxxxxxxxxx
>> cc: io-uring@xxxxxxxxxxxxxxx
>> ---
>> arch/x86/kernel/process.c | 49 +++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 49 insertions(+)
>>
>> diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
>> index 9c214d7085a4..72120c4b7618 100644
>> --- a/arch/x86/kernel/process.c
>> +++ b/arch/x86/kernel/process.c
>> @@ -163,6 +163,55 @@ int copy_thread(unsigned long clone_flags, unsigned long sp, unsigned long arg,
>> /* Kernel thread ? */
>> if (unlikely(p->flags & (PF_KTHREAD | PF_IO_WORKER))) {
>> memset(childregs, 0, sizeof(struct pt_regs));
>> + /*
>> + * gdb sees all userspace threads,
>> + * including io threads (PF_IO_WORKER)!
>> + *
>> + * gdb uses:
>> + * PTRACE_PEEKUSR, offsetof (struct user_regs_struct, cs)
>> + * returning with 0x33 (51) to detect 64 bit
>> + * and:
>> + * PTRACE_PEEKUSR, offsetof (struct user_regs_struct, ds)
>> + * returning 0x2b (43) to detect 32 bit.
>> + *
>> + * GDB relies on that the kernel returns the
>> + * same values for all threads, which means
>> + * we don't zero these out.
>> + *
>> + * Note that CONFIG_X86_64 handles 'es' and 'ds'
>> + * differently, see the following above:
>> + * savesegment(es, p->thread.es);
>> + * savesegment(ds, p->thread.ds);
>> + * and the CONFIG_X86_64 version of get_segment_reg().
>> + *
>> + * Linus proposed something like this:
>> + * (https://lore.kernel.org/io-uring/CAHk-=whEObPkZBe4766DmR46-=5QTUiatWbSOaD468eTgYc1tg@xxxxxxxxxxxxxx/)
>> + *
>> + * childregs->cs = __USER_CS;
>> + * childregs->ss = __USER_DS;
>> + * childregs->ds = __USER_DS;
>> + * childregs->es = __USER_DS;
>> + *
>> + * might make sense (just do it unconditionally, rather than making it
>> + * special to PF_IO_WORKER).
>> + *
>> + * But that doesn't make gdb happy in all cases.
>> + *
>> + * While 32bit userspace on a 64bit kernel is legacy,
>> + * it's still useful to allow 32bit libraries or nss modules
>> + * use the same code as the 64bit version of that library, which
>> + * can use io-uring just fine.
Whoa there! Can we take a big step back?
I saw all the hubbub about making io threads visible to gdb. Fine, but why do we allow gdb to read and write their register files at all? They *don’t have user state* because they *are not user threads*. Beyond that, Linux does not really have a concept of a 32-bit thread and a 64-bit thread. I realize that gdb does have this concept, but gdb is *wrong*, and it regularly causes problems when debugging mixed-mode programs or VMs.
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;
Any other solution results in all kinds of nasty questions. For example, kernel threads don’t have FPU state — what happens if gdb tries to access FPU state? What happens if gdb tries to *allocate* AMX state for an io_uring thread? What happens if the various remote arch_prctl accessors are used?
—Andy
