Re: [PATCH] x86/suspend: fix false positive KASAN warning on suspend/resume

From	Dmitry Vyukov <dvyukov@google.com>
Newsgroups	linux.kernel
Subject	Re: [PATCH] x86/suspend: fix false positive KASAN warning on suspend/resume
Date	2016-12-01 18:00 +0100
Message-ID	<sJCSe-6Dp-33@gated-at.bofh.it> (permalink)
References	(2 earlier) <sJiqu-1CH-13@gated-at.bofh.it> <sJmkp-4c1-1@gated-at.bofh.it> <sJB05-5a9-17@gated-at.bofh.it> <sJB05-5a9-15@gated-at.bofh.it> <sJCIx-6wt-5@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

On Thu, Dec 1, 2016 at 5:45 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> 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.bauer@intel.com>
>> > > Tested-by: Scott Bauer <scott.bauer@intel.com>
>> > > Signed-off-by: Josh Poimboeuf <jpoimboe@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.

restore_processor_state/__restore_processor_state does not seem to
have any local variables, so KASAN does not do any stack checks there.
We could disable KASAN instrumentation of the file, or of particular
functions. Or we could call kasan_unpoison_shadow() on the stack range
before switching to it.

Thread

BUG: KASAN: stack-out-of-bounds in unwind_get_return_address Scott Bauer <scott.bauer@intel.com> - 2016-11-29 19:30 +0100
  Re: BUG: KASAN: stack-out-of-bounds in unwind_get_return_address Josh Poimboeuf <jpoimboe@redhat.com> - 2016-11-30 19:40 +0100
    Re: BUG: KASAN: stack-out-of-bounds in unwind_get_return_address Scott Bauer <scott.bauer@intel.com> - 2016-11-30 20:10 +0100
      [PATCH] x86/suspend: fix false positive KASAN warning on  suspend/resume Josh Poimboeuf <jpoimboe@redhat.com> - 2016-12-01 00:20 +0100
        Re: [PATCH] x86/suspend: fix false positive KASAN warning on suspend/resume "Rafael J. Wysocki" <rafael@kernel.org> - 2016-12-01 15:10 +0100
          Re: [PATCH] x86/suspend: fix false positive KASAN warning on  suspend/resume Josh Poimboeuf <jpoimboe@redhat.com> - 2016-12-01 18:00 +0100
            Re: [PATCH] x86/suspend: fix false positive KASAN warning on suspend/resume "Rafael J. Wysocki" <rafael@kernel.org> - 2016-12-01 18:10 +0100
              Re: [PATCH] x86/suspend: fix false positive KASAN warning on  suspend/resume Ingo Molnar <mingo@kernel.org> - 2016-12-02 11:20 +0100
        Re: [PATCH] x86/suspend: fix false positive KASAN warning on  suspend/resume Josh Poimboeuf <jpoimboe@redhat.com> - 2016-12-01 16:00 +0100
          Re: [PATCH] x86/suspend: fix false positive KASAN warning on  suspend/resume Josh Poimboeuf <jpoimboe@redhat.com> - 2016-12-01 17:50 +0100
            Re: [PATCH] x86/suspend: fix false positive KASAN warning on suspend/resume Dmitry Vyukov <dvyukov@google.com> - 2016-12-01 18:00 +0100
              Re: [PATCH] x86/suspend: fix false positive KASAN warning on  suspend/resume Josh Poimboeuf <jpoimboe@redhat.com> - 2016-12-01 18:20 +0100
                Re: [PATCH] x86/suspend: fix false positive KASAN warning on suspend/resume Dmitry Vyukov <dvyukov@google.com> - 2016-12-01 18:30 +0100
                Re: [PATCH] x86/suspend: fix false positive KASAN warning on  suspend/resume Josh Poimboeuf <jpoimboe@redhat.com> - 2016-12-01 18:40 +0100
                Re: [PATCH] x86/suspend: fix false positive KASAN warning on suspend/resume Dmitry Vyukov <dvyukov@google.com> - 2016-12-01 18:50 +0100
                Re: [PATCH] x86/suspend: fix false positive KASAN warning on  suspend/resume Josh Poimboeuf <jpoimboe@redhat.com> - 2016-12-01 19:00 +0100
                [PATCH v2] x86/suspend: fix false positive KASAN warning on  suspend/resume Josh Poimboeuf <jpoimboe@redhat.com> - 2016-12-01 21:40 +0100
                Re: [PATCH v2] x86/suspend: fix false positive KASAN warning on suspend/resume Dmitry Vyukov <dvyukov@google.com> - 2016-12-02 10:50 +0100
                Re: [PATCH v2] x86/suspend: fix false positive KASAN warning on  suspend/resume Pavel Machek <pavel@ucw.cz> - 2016-12-02 14:00 +0100
                Re: [PATCH v2] x86/suspend: fix false positive KASAN warning on suspend/resume Dmitry Vyukov <dvyukov@google.com> - 2016-12-02 15:10 +0100
                Re: [PATCH v2] x86/suspend: fix false positive KASAN warning on  suspend/resume Josh Poimboeuf <jpoimboe@redhat.com> - 2016-12-02 15:10 +0100
                [PATCH v3] x86/suspend: fix false positive KASAN warning on  suspend/resume Josh Poimboeuf <jpoimboe@redhat.com> - 2016-12-02 15:50 +0100
                Re: [PATCH v3] x86/suspend: fix false positive KASAN warning on  suspend/resume Josh Poimboeuf <jpoimboe@redhat.com> - 2016-12-02 16:10 +0100
                [PATCH v4] x86/suspend: fix false positive KASAN warning on  suspend/resume Josh Poimboeuf <jpoimboe@redhat.com> - 2016-12-02 18:50 +0100
                Re: [PATCH v4] x86/suspend: fix false positive KASAN warning on  suspend/resume Pavel Machek <pavel@ucw.cz> - 2016-12-02 22:10 +0100
                Re: [PATCH v4] x86/suspend: fix false positive KASAN warning on  suspend/resume Josh Poimboeuf <jpoimboe@redhat.com> - 2016-12-02 23:00 +0100
                Re: [PATCH v4] x86/suspend: fix false positive KASAN warning on suspend/resume "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-12-08 01:20 +0100

csiph-web