From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk0-f199.google.com (mail-qk0-f199.google.com [209.85.220.199]) by kanga.kvack.org (Postfix) with ESMTP id 0763E6B0007 for ; Thu, 8 Feb 2018 12:20:29 -0500 (EST) Received: by mail-qk0-f199.google.com with SMTP id d131so4203659qkc.16 for ; Thu, 08 Feb 2018 09:20:29 -0800 (PST) Received: from mx1.redhat.com (mx3-rdu2.redhat.com. [66.187.233.73]) by mx.google.com with ESMTPS id r17si409822qtn.202.2018.02.08.09.20.28 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 08 Feb 2018 09:20:28 -0800 (PST) Date: Thu, 8 Feb 2018 11:20:26 -0600 From: Josh Poimboeuf Subject: Re: [PATCH RFC] x86: KASAN: Sanitize unauthorized irq stack access Message-ID: <20180208172026.6kqimndwyekyzzvl@treble> References: <151802005995.4570.824586713429099710.stgit@localhost.localdomain> <6638b09b-30b0-861e-9c00-c294889a3791@linux.intel.com> <20180208163041.zy7dbz4tlbit4i2h@treble> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Sender: owner-linux-mm@kvack.org List-ID: To: Dmitry Vyukov Cc: Kirill Tkhai , Dave Hansen , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , Andrey Ryabinin , Alexander Potapenko , Andy Lutomirski , Borislav Petkov , Juergen Gross , "Kirill A. Shutemov" , Kees Cook , Mathias Krause , Greg Kroah-Hartman , Kate Stewart , LKML , kasan-dev , Linux-MM On Thu, Feb 08, 2018 at 05:41:19PM +0100, Dmitry Vyukov wrote: > On Thu, Feb 8, 2018 at 5:30 PM, Josh Poimboeuf wrote: > > On Thu, Feb 08, 2018 at 01:03:49PM +0300, Kirill Tkhai wrote: > >> On 07.02.2018 21:38, Dave Hansen wrote: > >> > On 02/07/2018 08:14 AM, Kirill Tkhai wrote: > >> >> Sometimes it is possible to meet a situation, > >> >> when irq stack is corrupted, while innocent > >> >> callback function is being executed. This may > >> >> happen because of crappy drivers irq handlers, > >> >> when they access wrong memory on the irq stack. > >> > > >> > Can you be more clear about the actual issue? Which drivers do this? > >> > How do they even find an IRQ stack pointer? > >> > >> I can't say actual driver making this, because I'm still investigating the guilty one. > >> But I have couple of crash dumps with the crash inside update_sd_lb_stats() function, > >> where stack variable sg becomes corrupted. This time all scheduler-related not-stack > >> variables are in ideal state. And update_sd_lb_stats() is the function, which can't > >> corrupt its own stack. So, I thought this functionality may be useful for something else, > >> especially because of irq stack is one of the last stacks, which are not sanitized. > >> Task's stacks are already covered, as I know > >> > >> [1595450.678971] Call Trace: > >> [1595450.683991] > >> [1595450.684038] > >> [1595450.688926] [] cpumask_next_and+0x35/0x50 > >> [1595450.693984] [] find_busiest_group+0x143/0x950 > >> [1595450.699088] [] load_balance+0x19a/0xc20 > >> [1595450.704289] [] ? sched_clock_cpu+0x85/0xc0 > >> [1595450.709457] [] ? update_rq_clock.part.88+0x1a/0x150 > >> [1595450.714711] [] rebalance_domains+0x170/0x2b0 > >> [1595450.719997] [] run_rebalance_domains+0x122/0x1e0 > >> [1595450.725321] [] __do_softirq+0x10f/0x2aa > >> [1595450.730746] [] call_softirq+0x1c/0x30 > >> [1595450.736169] [] do_softirq+0x65/0xa0 > >> [1595450.741754] [] irq_exit+0x105/0x110 > >> [1595450.747279] [] smp_apic_timer_interrupt+0x42/0x50 > >> [1595450.752905] [] apic_timer_interrupt+0x232/0x240 > >> [1595450.758519] > >> [1595450.758569] > >> [1595450.764100] [] ? cpuidle_enter_state+0x52/0xc0 > >> [1595450.769652] [] cpuidle_idle_call+0xd8/0x210 > >> [1595450.775198] [] arch_cpu_idle+0xe/0x30 > >> [1595450.780813] [] cpu_startup_entry+0x14a/0x1c0 > >> [1595450.786286] [] start_secondary+0x1d6/0x250 > > > > I'm not seeing how this patch would help. If you're running on the irq > > stack, the *entire* irq stack would be unpoisoned. So there's still no > > KASAN protection. Or am I missing something? > > > > Seems like it would be more useful for KASAN to detect redzone accesses > > on the irq stack (if it's not doing that already). > > KASAN should do this already (unless there is something terribly > broken). Compiler instrumentation adds redzones around all stack > variables and injects code to poision/unpoison these redzones on > function entry/exit. > KASAN can also detect use-after-scope bugs for stack variables, but > this requires a more recent gcc (6 or 7, don't remember exactly now) > and CONFIG_KASAN_EXTRA since recently. > User-space ASAN can also detect so called use-after-return bugs > (dangling references to stack variables), but this requires manual > management of stack frames and quarantine for stack frames. This is > more tricky to do inside of kernel, so this was never implemented in > KASAN. KASAN still can detect some of these, if it will happen so that > the dangling reference happen to point to a redzone in a new frame. Ok, that's good. And it seems this patch doesn't change that. So it looks like the purpose of the patch is to protect the irq stack from code which is *not* running on the irq stack. Which seems a bit far-fetched and theoretical. Though I don't see any harm in it. The patch description is confusing. It talks about "crappy drivers irq handlers when they access wrong memory on the stack". But if I understand correctly, the patch doesn't actually protect against that case, because irq handlers run on the irq stack, and this patch only affects code which *isn't* running on the irq stack. -- Josh -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org