linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Josh Poimboeuf <jpoimboe@redhat.com>
To: Andrey Ryabinin <aryabinin@virtuozzo.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Len Brown <len.brown@intel.com>, Pavel Machek <pavel@ucw.cz>,
	linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
	peterz@infradead.org, mingo@kernel.org, luto@amacapital.net,
	Scott Bauer <scott.bauer@intel.com>,
	x86@kernel.org, Alexander Potapenko <glider@google.com>,
	Dmitry Vyukov <dvyukov@google.com>,
	kasan-dev@googlegroups.com
Subject: Re: [PATCH] x86/suspend: fix false positive KASAN warning on suspend/resume
Date: Thu, 1 Dec 2016 08:58:21 -0600	[thread overview]
Message-ID: <20161201145821.imkcgizo4thmiei2@treble> (raw)
In-Reply-To: <8f4c4a62-d912-0cd9-3462-8df20a868834@virtuozzo.com>

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.

-- 
Josh

  reply	other threads:[~2016-12-01 14:58 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-29 18:13 BUG: KASAN: stack-out-of-bounds in unwind_get_return_address Scott Bauer
2016-11-30 18:35 ` Josh Poimboeuf
2016-11-30 19:02   ` Scott Bauer
2016-11-30 23:10     ` [PATCH] x86/suspend: fix false positive KASAN warning on suspend/resume Josh Poimboeuf
2016-12-01  9:05       ` Andrey Ryabinin
2016-12-01 14:58         ` Josh Poimboeuf [this message]
2016-12-01 16:45           ` Josh Poimboeuf
2016-12-01 16:51             ` Dmitry Vyukov
2016-12-01 17:13               ` Josh Poimboeuf
2016-12-01 17:27                 ` Dmitry Vyukov
2016-12-01 17:34                   ` Josh Poimboeuf
2016-12-01 17:47                     ` Dmitry Vyukov
2016-12-01 17:56                       ` Josh Poimboeuf
2016-12-01 20:31                         ` [PATCH v2] " Josh Poimboeuf
2016-12-02  9:44                           ` Dmitry Vyukov
2016-12-02 12:54                           ` Pavel Machek
2016-12-02 13:41                           ` Andrey Ryabinin
2016-12-02 14:01                             ` Josh Poimboeuf
2016-12-02 14:02                               ` Dmitry Vyukov
2016-12-02 14:42                               ` [PATCH v3] " Josh Poimboeuf
2016-12-02 14:45                                 ` Andrey Ryabinin
2016-12-02 15:08                                   ` Josh Poimboeuf
2016-12-02 17:42                                     ` [PATCH v4] " Josh Poimboeuf
2016-12-02 20:55                                       ` Andrey Ryabinin
2016-12-02 21:09                                       ` Pavel Machek
2016-12-02 21:57                                         ` Josh Poimboeuf
2016-12-08  0:10                                       ` Rafael J. Wysocki
2016-12-01 14:04       ` [PATCH] " Rafael J. Wysocki
2016-12-01 16:53         ` Josh Poimboeuf
2016-12-01 17:05           ` Rafael J. Wysocki
2016-12-02 10:15             ` Ingo Molnar

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=20161201145821.imkcgizo4thmiei2@treble \
    --to=jpoimboe@redhat.com \
    --cc=aryabinin@virtuozzo.com \
    --cc=dvyukov@google.com \
    --cc=glider@google.com \
    --cc=kasan-dev@googlegroups.com \
    --cc=len.brown@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=mingo@kernel.org \
    --cc=pavel@ucw.cz \
    --cc=peterz@infradead.org \
    --cc=rjw@rjwysocki.net \
    --cc=scott.bauer@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 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).