From: Josh Poimboeuf <jpoimboe@redhat.com>
To: Andy Lutomirski <luto@amacapital.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
Ingo Molnar <mingo@kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Dave Hansen <dave.hansen@linux.intel.com>,
Thomas Gleixner <tglx@linutronix.de>,
"H . Peter Anvin" <hpa@zytor.com>,
Peter Zijlstra <peterz@infradead.org>,
Borislav Petkov <bp@alien8.de>
Subject: Re: [PATCH 11/21] x86/dumpstack: Handle stack overflow on all stacks
Date: Tue, 28 Nov 2017 12:15:54 -0600 [thread overview]
Message-ID: <20171128181554.jl2qp7ca2tgfnuiq@treble> (raw)
In-Reply-To: <CALCETrXptow2p4N4zqCny-inHyEcUw4hemDnvw50i5BcKx0G6w@mail.gmail.com>
On Mon, Nov 27, 2017 at 09:29:26PM -0800, Andy Lutomirski wrote:
> On Mon, Nov 27, 2017 at 8:29 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > On Mon, Nov 27, 2017 at 11:26:30AM -0800, Linus Torvalds wrote:
> >> On Mon, Nov 27, 2017 at 2:45 AM, Ingo Molnar <mingo@kernel.org> wrote:
> >> > From: Andy Lutomirski <luto@kernel.org>
> >> >
> >> > We currently special-case stack overflow on the task stack. We're
> >> > going to start putting special stacks in the fixmap with a custom
> >> > layout, so they'll have guard pages, too. Teach the unwinder to be
> >> > able to unwind an overflow of any of the stacks.
> >>
> >> Why isn't this together with 01/21? The two cases seem to be entirely
> >> identical and fundamentally the same issue.
> >
> > Yeah, they probably do belong in the same patch.
> >
> >> In fact, maybe the whole "stack overflow" special cases should be in
> >> "get_stack_info()" itself, rather than be special-cased in the
> >> callers?
> >
> > I would be nervous about doing that. Several of the get_stack_info()
> > callers rely on it being honest.
> >
> > In fact, looking deeper at the above patch, it doesn't seem convincingly
> > safe to me. What if the adjacent page doesn't exist? Then when the
> > oops dumping code dereferences the 'stack' variable, you get an oops in
> > your oops.
I was wrong here. Too much thinking, not enough sleep. Looking at it
today, the patch looks safe again.
The 'stack' variable is actually safe to dereference afterwards because
it gets updated with the safe aligned address.
> Isn't the oops dumping code supposed to dereference everything using a
> special safe function?
That's not what it does now. The 'stack' pointer variable is
dereferenced normally, unless KASAN is enabled. It relies on
get_stack_info() doing the safety check.
> Anyway, get_stack_info() wouldn't really be lying. It would just be
> returning something where begin..end doesn't contain the requested
> pointer.
At the very least, it would be surprising. I prefer the current
approach, it's much more explicit.
--
Josh
next prev parent reply other threads:[~2017-11-28 18:16 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-27 10:45 [PATCH 00/21] Preparatory patches for x86 KAISER support Ingo Molnar
2017-11-27 10:45 ` [PATCH 01/21] x86/unwinder/orc: Don't bail on stack overflow Ingo Molnar
2017-11-27 14:47 ` Borislav Petkov
2017-11-27 10:45 ` [PATCH 02/21] x86/unwinder: Handle stack overflows more gracefully Ingo Molnar
2017-11-27 17:36 ` Borislav Petkov
2017-11-28 4:55 ` Josh Poimboeuf
2017-11-27 10:45 ` [PATCH 03/21] x86/irq: Remove an old outdated comment about context tracking races Ingo Molnar
2017-11-27 10:45 ` [PATCH 04/21] x86/irq/64: Print the offending IP in the stack overflow warning Ingo Molnar
2017-11-27 10:45 ` [PATCH 05/21] x86/entry/64: Allocate and enable the SYSENTER stack Ingo Molnar
2017-11-27 10:45 ` [PATCH 06/21] x86/dumpstack: Add get_stack_info() support for " Ingo Molnar
2017-11-27 10:45 ` [PATCH 07/21] x86/entry/gdt: Put per-CPU GDT remaps in ascending order Ingo Molnar
2017-11-27 10:45 ` [PATCH 08/21] x86/mm/fixmap: Generalize the GDT fixmap mechanism, introduce 'struct cpu_entry_area' Ingo Molnar
2017-11-27 10:45 ` [PATCH 09/21] x86/kasan/64: Teach KASAN about the cpu_entry_area Ingo Molnar
2017-11-27 10:45 ` [PATCH 10/21] x86/entry: Fix assumptions that the HW TSS is at the beginning of cpu_tss Ingo Molnar
2017-11-27 10:45 ` [PATCH 11/21] x86/dumpstack: Handle stack overflow on all stacks Ingo Molnar
2017-11-27 19:26 ` Linus Torvalds
2017-11-28 4:29 ` Josh Poimboeuf
2017-11-28 5:29 ` Andy Lutomirski
2017-11-28 18:15 ` Josh Poimboeuf [this message]
2017-11-27 10:45 ` [PATCH 12/21] x86/entry: Move SYSENTER_stack to the beginning of struct tss_struct Ingo Molnar
2017-11-27 10:45 ` [PATCH 13/21] x86/entry: Remap the TSS into the CPU entry area Ingo Molnar
2017-11-27 10:45 ` [PATCH 14/21] x86/entry/64: Separate cpu_current_top_of_stack from TSS.sp0 Ingo Molnar
2017-11-27 10:45 ` [PATCH 15/21] x86/espfix/64: Stop assuming that pt_regs is on the entry stack Ingo Molnar
2017-11-27 10:45 ` [PATCH 16/21] x86/entry/64: Use a per-CPU trampoline stack for IDT entries Ingo Molnar
2017-12-01 17:06 ` Dave Hansen
2017-12-01 17:44 ` Andy Lutomirski
2017-12-01 21:21 ` Dave Hansen
2017-12-01 21:59 ` Andy Lutomirski
2017-12-02 6:41 ` Kevin Easton
2017-11-27 10:45 ` [PATCH 17/21] x86/entry/64: Return to userspace from the trampoline stack Ingo Molnar
2017-11-27 10:45 ` [PATCH 18/21] x86/entry/64: Create a per-CPU SYSCALL entry trampoline Ingo Molnar
2017-11-27 10:45 ` [PATCH 19/21] x86/entry/64: Move the IST stacks into 'struct cpu_entry_area' Ingo Molnar
2017-11-27 10:45 ` [PATCH 20/21] x86/entry/64: Remove the SYSENTER stack canary Ingo Molnar
2017-11-27 10:45 ` [PATCH 21/21] x86/entry: Clean up the SYSENTER_stack code Ingo Molnar
2017-12-01 17:59 ` Borislav Petkov
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=20171128181554.jl2qp7ca2tgfnuiq@treble \
--to=jpoimboe@redhat.com \
--cc=bp@alien8.de \
--cc=dave.hansen@linux.intel.com \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@amacapital.net \
--cc=mingo@kernel.org \
--cc=peterz@infradead.org \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).