From: Ingo Molnar <mingo@kernel.org>
To: Sean Christopherson <sean.j.christopherson@intel.com>
Cc: Dave Hansen <dave.hansen@intel.com>,
Changbin Du <changbin.du@gmail.com>,
Dave Hansen <dave.hansen@linux.intel.com>,
Andy Lutomirski <luto@kernel.org>,
x86@kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] x86/mm: determine whether the fault address is canonical
Date: Mon, 7 Oct 2019 16:32:55 +0200 [thread overview]
Message-ID: <20191007143255.GA59713@gmail.com> (raw)
In-Reply-To: <20191004153115.GA19503@linux.intel.com>
* Sean Christopherson <sean.j.christopherson@intel.com> wrote:
> On Fri, Oct 04, 2019 at 07:39:08AM -0700, Dave Hansen wrote:
> > On 10/4/19 6:45 AM, Changbin Du wrote:
> > > +static inline bool is_canonical_addr(u64 addr)
> > > +{
> > > +#ifdef CONFIG_X86_64
> > > + int shift = 64 - boot_cpu_data.x86_phys_bits;
> >
> > I think you mean to check the virtual bits member, not "phys_bits".
> >
> > BTW, I also prefer the IS_ENABLED(CONFIG_) checks to explicit #ifdefs.
> > Would one of those work in this case?
> >
> > As for the error message:
> >
> > > {
> > > - WARN_ONCE(trapnr == X86_TRAP_GP, "General protection fault in user access. Non-canonical address?");
> > > + WARN_ONCE(trapnr == X86_TRAP_GP, "General protection fault at %s address in user access.",
> > > + is_canonical_addr(fault_addr) ? "canonical" : "non-canonical");
> >
> > I've always read that as "the GP might have been caused by a
> > non-canonical access". The main nit I'd have with the change is that I
> > don't think all #GP's during user access functions which are given a
> > non-canonical address *necessarily* caused the #GP.
> >
> > There are a billion ways you can get a #GP and I bet canonical
> > violations aren't the only way you can get one in a user copy function.
>
> All the other reasons would require a fairly egregious kernel bug, hence
> the speculation that the #GP is due to a non-canonical address. Something
> like the following would be more precise, though highly unlikely to ever
> be exercised, e.g. KVM had a fatal bug related to injecting a non-zero
> error code that went unnoticed for years.
>
> WARN_ONCE(trapnr == X86_TRAP_GP, "General protection fault in user access. %s?\n",
> (IS_ENABLED(CONFIG_X86_64) && !error_code) ? "Non-canonical address" :
> "Segmentation bug");
Instead of trying to guess the reason of the #GPF (which guess might be
wrong), please just state it as the reason if we are sure that the cause
is a non-canonical address - and provide a best-guess if it's not but
clearly signal that it's a guess.
I.e. if I understood all the cases correctly we'd have three types of
messages generated:
!error_code:
"General protection fault in user access, due to non-canonical address."
error_code && !is_canonical_addr(fault_addr):
"General protection fault in user access. Non-canonical address?"
error_code && is_canonical_addr(fault_addr):
"General protection fault in user access. Segmentation bug?"
Only the first one is declarative, because we know we got a #GP with a
zero error code which should denote a non-canonical address access.
The second and third ones are guesses with question marks to communicate
the uncertainty.
Assuming that !error_code always means non-canonical access?
And hopefully "!error_code && !is_canonical_addr(fault_addr)" is not
possible?
Thanks,
Ingo
next prev parent reply other threads:[~2019-10-07 14:33 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-04 13:45 [PATCH] x86/mm: determine whether the fault address is canonical Changbin Du
2019-10-04 14:39 ` Dave Hansen
2019-10-04 15:31 ` Sean Christopherson
2019-10-07 14:32 ` Ingo Molnar [this message]
2019-10-07 14:44 ` Ingo Molnar
2019-10-07 15:13 ` Sean Christopherson
2019-10-04 14:59 ` Andy Lutomirski
2019-10-04 15:14 ` Dave Hansen
2019-10-06 2:29 ` Changbin Du
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 \
--in-reply-to=20191007143255.GA59713@gmail.com \
--to=mingo@kernel.org \
--cc=changbin.du@gmail.com \
--cc=dave.hansen@intel.com \
--cc=dave.hansen@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@kernel.org \
--cc=sean.j.christopherson@intel.com \
--cc=x86@kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.