From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934428AbcECSTk (ORCPT ); Tue, 3 May 2016 14:19:40 -0400 Received: from mail.linuxfoundation.org ([140.211.169.12]:56473 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932756AbcECSTi (ORCPT ); Tue, 3 May 2016 14:19:38 -0400 Date: Tue, 3 May 2016 11:19:37 -0700 From: Greg Kroah-Hartman To: Michael Neuling Cc: Jiri Slaby , linux-kernel@vger.kernel.org, stable@vger.kernel.org, Michael Ellerman Subject: Re: [PATCH 4.4 60/67] powerpc/tm: Check for already reclaimed tasks Message-ID: <20160503181937.GB7819@kroah.com> References: <20160127180907.419868641@linuxfoundation.org> <20160127180913.484826415@linuxfoundation.org> <57284604.5070408@suse.cz> <1462273457.4115.29.camel@neuling.org> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1462273457.4115.29.camel@neuling.org> User-Agent: Mutt/1.6.1 (2016-04-27) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, May 03, 2016 at 09:04:17PM +1000, Michael Neuling wrote: > On Tue, 2016-05-03 at 08:32 +0200, Jiri Slaby wrote: > > On 01/27/2016, 07:12 PM, Greg Kroah-Hartman wrote: > > > > > > 4.4-stable review patch.  If anyone has any objections, please let me > > > know. > > > > > > ------------------ > > > > > > From: Michael Neuling > > > > > > commit 7f821fc9c77a9b01fe7b1d6e72717b33d8d64142 upstream. > > > > > > Currently we can hit a scenario where we'll tm_reclaim() twice.  This > > > results in a TM bad thing exception because the second reclaim occurs > > > when not in suspend mode. > > > > > > The scenario in which this can happen is the following.  We attempt to > > > deliver a signal to userspace.  To do this we need obtain the stack > > > pointer to write the signal context.  To get this stack pointer we > > > must tm_reclaim() in case we need to use the checkpointed stack > > > pointer (see get_tm_stackpointer()).  Normally we'd then return > > > directly to userspace to deliver the signal > > > without going through > > > __switch_to(). > > > > > > Unfortunatley, if at this point we get an error (such as a bad > > > userspace stack pointer), we need to exit the process.  The exit will > > > result in a __switch_to().  __switch_to() will attempt to save the > > > process state which results in another tm_reclaim().  This > > > tm_reclaim() now causes a TM Bad Thing exception as this state has > > > already been saved and the processor is no longer in TM suspend mode. > > > Whee! > > > > > > This patch checks the state of the MSR to ensure we are TM suspended > > > before we attempt the tm_reclaim().  If we've already saved the state > > > away, we should no longer be in TM suspend mode.  This has the > > > additional advantage of checking for a potential TM Bad Thing > > > exception. > > > > > > Found using syscall fuzzer. > > > > > > Fixes: fb09692e71f1 ("powerpc: Add reclaim and recheckpoint functions > > > for context switching transactional memory processes") > > > Signed-off-by: Michael Neuling > > > Signed-off-by: Michael Ellerman > > > Signed-off-by: Greg Kroah-Hartman > > > > > > --- > > >  arch/powerpc/kernel/process.c |   18 ++++++++++++++++++ > > >  1 file changed, 18 insertions(+) > > > > > > --- a/arch/powerpc/kernel/process.c > > > +++ b/arch/powerpc/kernel/process.c > > > @@ -569,6 +569,24 @@ static void tm_reclaim_thread(struct thr > > >   if (!MSR_TM_SUSPENDED(mfmsr())) > > >   return; > > >   > > > + /* > > > +  * Use the current MSR TM suspended bit to track if we have > > > +  * checkpointed state outstanding. > > > +  * On signal delivery, we'd normally reclaim the checkpointed > > > +  * state to obtain stack pointer (see:get_tm_stackpointer()). > > > +  * This will then directly return to userspace without going > > > +  * through __switch_to(). However, if the stack frame is bad, > > > +  * we need to exit this thread which calls __switch_to() which > > > +  * will again attempt to reclaim the already saved tm state. > > > +  * Hence we need to check that we've not already reclaimed > > > +  * this state. > > > +  * We do this using the current MSR, rather tracking it in > > > +  * some specific thread_struct bit, as it has the additional > > > +  * benifit of checking for a potential TM bad thing exception. > > > +  */ > > > + if (!MSR_TM_SUSPENDED(mfmsr())) > > > + return; > >  > > This one should have not been applied to 4.4. The patch is in mainline > > since 4.4-rc6. Hence the check is duplicated as can be seen above. > > Greg, surely your scripts could check for that? My "scripts" are usually me looking at them manually, I messed up here, sorry. I'll revert it in the next round of stable kernel releases after this one. thanks, greg k-h