On Thu, Dec 01, 2016 at 08:58:21AM -0600, Josh Poimboeuf wrote: > On Thu, Dec 01, 2016 at 12:05:34PM +0300, Andrey Ryabinin wrote: > > > > > > On 12/01/2016 02:10 AM, Josh Poimboeuf wrote: > > > Resuming from a suspend operation is showing a KASAN false positive > > > warning: > > > > > > > > KASAN instrumentation poisons the stack when entering a function and > > > unpoisons it when exiting the function. However, in the suspend path, > > > some functions never return, so their stack never gets unpoisoned, > > > resulting in stale KASAN shadow data which can cause false positive > > > warnings like the one above. > > > > > > Reported-by: Scott Bauer <scott.ba...@intel.com> > > > Tested-by: Scott Bauer <scott.ba...@intel.com> > > > Signed-off-by: Josh Poimboeuf <jpoim...@redhat.com> > > > --- > > > arch/x86/kernel/acpi/sleep.c | 3 +++ > > > include/linux/kasan.h | 7 +++++++ > > > 2 files changed, 10 insertions(+) > > > > > > diff --git a/arch/x86/kernel/acpi/sleep.c b/arch/x86/kernel/acpi/sleep.c > > > index 4858733..62bd046 100644 > > > --- a/arch/x86/kernel/acpi/sleep.c > > > +++ b/arch/x86/kernel/acpi/sleep.c > > > @@ -115,6 +115,9 @@ int x86_acpi_suspend_lowlevel(void) > > > pause_graph_tracing(); > > > do_suspend_lowlevel(); > > > unpause_graph_tracing(); > > > + > > > + kasan_unpoison_stack_below_sp(); > > > + > > > > I think this might be too late. We may hit stale poison in the first C > > function called > > after resume (restore_processor_state()). Thus the shadow must be > > unpoisoned prior such call, > > i.e. somewhere in do_suspend_lowlevel() after .Lresume_point. > > Yeah, I think you're right. Will spin a v2.
So I tried calling kasan_unpoison_task_stack_below() from do_suspend_lowlevel(), but it hung on the resume. Presumably because restore_processor_state() does some important setup which would be needed before calling into kasan_unpoison_task_stack_below(). For example, setting up the gs register. So it's a bit of a catch-22. It could probably be fixed properly by rewriting do_suspend_lowlevel() to call restore_processor_state() with the temporary stack before switching to the original stack and doing the unpoison. (And there are some other issues with do_suspend_lowlevel() and I'd love to try taking a scalpel to it. But I have too many knives in the air already to want to try to attempt that right now...) Unless somebody else wants to take a stab at it, my original patch is probably good enough for now, since restore_processor_state() doesn't seem to be triggering any KASAN warnings. -- Josh
