On Mon, 2015-11-16 at 12:51 +0530, Anshuman Khandual wrote: > On 11/13/2015 10:27 AM, Michael Neuling wrote: > > 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. > > Can this situation be created using a test and verified that with > this new change, the kernel can handle it successfully. I guess > the self test in the series does not cover this scenario.
No it doesn't. The syscall fuzzer I have does hit it but I don't have permission to post that. > > > > Found using syscall fuzzer. > > > > Signed-off-by: Michael Neuling <mi...@neuling.org> > > Cc: sta...@vger.kernel.org > > --- > > arch/powerpc/kernel/process.c | 19 +++++++++++++++++++ > > 1 file changed, 19 insertions(+) > > > > diff --git a/arch/powerpc/kernel/process.c > > b/arch/powerpc/kernel/process.c > > index 5fbe5d8..a1b41d1 100644 > > --- a/arch/powerpc/kernel/process.c > > +++ b/arch/powerpc/kernel/process.c > > @@ -551,6 +551,25 @@ static void tm_reclaim_thread(struct > > thread_struct *thr, > > msr_diff &= MSR_FP | MSR_VEC | MSR_VSX | MSR_FE0 | > > MSR_FE1; > > } > > > > + /* > > + * 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 bit thread_struct bit, as it has the > > There is one extra "bit" here ^^^^^. Thanks! Mikey > _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
