public inbox for [email protected]
 help / color / mirror / Atom feed
From: Simon Marchi <[email protected]>
To: Andy Lutomirski <[email protected]>
Cc: Stefan Metzmacher <[email protected]>,
	Borislav Petkov <[email protected]>,
	Peter Zijlstra <[email protected]>,
	Linus Torvalds <[email protected]>,
	Thomas Gleixner <[email protected]>,
	Jens Axboe <[email protected]>,
	Linux Kernel Mailing List <[email protected]>,
	io-uring <[email protected]>,
	the arch/x86 maintainers <[email protected]>,
	[email protected]
Subject: Re: [PATCH] io_thread/x86: don't reset 'cs', 'ss', 'ds' and 'es' registers for io_threads
Date: Wed, 5 May 2021 21:04:32 -0400	[thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <CALCETrUF5M+Qw+RfY8subR7nzmpMyFsE3NHSAPoMVWMz6_hr-w@mail.gmail.com>

On 2021-05-05 6:11 p.m., Andy Lutomirski wrote:
> For what it's worth, this is already fundamentally incorrect.  On
> x86_64 Linux, a process *does* *not* *have* an architecture.  Every
> task on an x86_64 Linux host has a full 64-bit register state.  The
> task can, and sometimes does, change CS using far transfers or other
> bizarre techniques, and neither the kernel nor GDB will be notified or
> have a chance to take any action in response.  ELF files can be
> 32-bit, CS:rIP can point at 32-bit code, and system calls can be
> 32-bit (even from 64-bit code), but *tasks* are not 32-bit.

Thanks for that explanation: I didn't know that "32-bit" tasks had the
same register state as any other task.  I never really looked into it,
but I was assuming that tasks were either 32-bit or 64-bit (based on the
ELF header of the file they exec'd or something) and that 32-bit tasks
had the register state as a task on an i386 machine would have.  And
that PEEKUSER would return the 64-bit register state for 64-bit tasks,
and 32-bit register state for 32-bit tasks.

I looked at how GDB reads registers from a "64-bit" task and a "32-bit"
task (I have to quote now, since I now know it's an abuse of
terminology) side by side.  And indeed, GDB reads a full 64-bit state in
both cases.  For the 32-bit case, it picks the 32-bit values from that
buffer.  For example, to get the eax value it picks the low 4 bytes of
rax (well, ax in user_regs_struct).

So I suppose that if GDB wanted to tell nothing but the truth, it would
present the full 64-bit register state to the user even when debugging a
32-bit program.  But at the end of the day, the typical user debugging a
32-bit program on a 64-bit probably just wants the illusion that they
are on i386.

> Now I realize that the ptrace() API is awful and makes life difficult
> in several respects for no good reason but, if gdb is ever interested
> in fixing its ideas about architecture to understand that all tasks,
> even those that think of themselves as "compat", have full 64-bit
> state, I would be more than willing to improve the ptrace() API as
> needed to make this work well.

Just wondering, do you have specific ptrace shortcomings in mind when
saying this?  As I found above, ptrace lets us read the whole 64-bit
register state.  After that it's up to us to analyze the state of the
program based on its registers and memory.  What more could ptrace give
us?

> Since I'm not holding my breath, please at least keep in mind that
> anything you do here is merely a heuristic, cannot be fully correct,
> and then whenever gdb determines that a thread group or a thread is
> "32-bit", gdb is actually deciding to operate in a degraded mode for
> that task, is not accurately representing the task state, and is at
> risk of crashing, malfunctioning, or crashing the inferior due to its
> incorrect assumptions.  If you have ever attached gdb to QEMU's
> gdbserver and tried to debug the early boot process of a 64-bit Linux
> kernel, you may have encountered this class of bugs.  gdb works very,
> very poorly for this use case.

Yes, that QEMU case comes up often.  I wish that things were better, but
the reality is that this is an edge case, it would require somebody with
that particular itch to scratch to work on GDB to improve that use case.
So as you said, don't hold your breath :).

I completely understand that GDB putting processes in the "32-bit" or
"64-bit" bin is not the right thing to do in general, from a kernel
perspective.  But it converged to this because it's enough for and
useful to the 99.9% of users who debug programs that don't do funky
things.  At least, it's good to know about it in case problems related
to this arise in the future.

Simon

  parent reply	other threads:[~2021-05-06  1:05 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <[email protected]>
2021-05-03 16:05 ` [PATCH] io_thread/x86: don't reset 'cs', 'ss', 'ds' and 'es' registers for io_threads Andy Lutomirski
2021-05-03 19:14   ` Linus Torvalds
2021-05-03 20:15     ` Andy Lutomirski
2021-05-03 20:21       ` Andy Lutomirski
2021-05-03 20:37       ` Linus Torvalds
2021-05-03 21:26         ` Jens Axboe
2021-05-03 21:49           ` Andy Lutomirski
2021-05-03 22:08             ` Linus Torvalds
2021-05-03 22:56               ` Thomas Gleixner
2021-05-03 23:15                 ` Andy Lutomirski
2021-05-03 23:16                 ` Linus Torvalds
2021-05-03 23:19                   ` Linus Torvalds
2021-05-03 23:27                   ` Stefan Metzmacher
2021-05-03 23:48                     ` Linus Torvalds
2021-05-04  2:50                       ` Jens Axboe
2021-05-04 11:39                         ` Stefan Metzmacher
2021-05-04 15:53                           ` Linus Torvalds
2021-05-12  4:24                         ` Olivier Langlois
2021-05-12 17:44                           ` Linus Torvalds
2021-05-12 20:55                             ` Jens Axboe
2021-05-20  4:13                               ` Olivier Langlois
2021-05-21  7:31                                 ` Olivier Langlois
2021-05-25 19:39                                   ` Olivier Langlois
2021-05-25 19:45                                     ` Olivier Langlois
2021-05-25 19:52                                     ` Jens Axboe
2021-05-25 20:23                                     ` Linus Torvalds
2021-05-04  8:22                       ` David Laight
2021-05-04  0:01                   ` Andy Lutomirski
2021-05-04  8:39     ` Peter Zijlstra
2021-05-04 15:35       ` Borislav Petkov
2021-05-04 15:55         ` Simon Marchi
2021-05-05 11:29           ` Stefan Metzmacher
2021-05-05 21:59             ` Simon Marchi
2021-05-05 22:11               ` Andy Lutomirski
2021-05-05 23:12                 ` Borislav Petkov
2021-05-05 23:22                   ` Andy Lutomirski
2021-05-06  1:04                 ` Simon Marchi [this message]
2021-05-06 15:11                   ` Andy Lutomirski
2021-05-06  9:47                 ` David Laight
2021-05-06  9:53                   ` David Laight
2021-05-05 22:21               ` Stefan Metzmacher
2021-05-05 23:15                 ` Simon Marchi
2021-04-11 15:27 Stefan Metzmacher
2021-04-14 21:28 ` Stefan Metzmacher

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox