On Wed, 2020-04-01 at 02:38:36 UTC, Michael Ellerman wrote: > In restore_tm_sigcontexts() we take the trap value directly from the > user sigcontext with no checking: > > err |= __get_user(regs->trap, &sc->gp_regs[PT_TRAP]); > > This means we can be in the kernel with an arbitrary regs->trap value. > > Although that's not immediately problematic, there is a risk we could > trigger one of the uses of CHECK_FULL_REGS(): > > #define CHECK_FULL_REGS(regs) BUG_ON(regs->trap & 1) > > It can also cause us to unnecessarily save non-volatile GPRs again in > save_nvgprs(), which shouldn't be problematic but is still wrong. > > It's also possible it could trick the syscall restart machinery, which > relies on regs->trap not being == 0xc00 (see 9a81c16b5275 ("powerpc: > fix double syscall restarts")), though I haven't been able to make > that happen. > > Finally it doesn't match the behaviour of the non-TM case, in > restore_sigcontext() which zeroes regs->trap. > > So change restore_tm_sigcontexts() to zero regs->trap. > > This was discovered while testing Nick's upcoming rewrite of the > syscall entry path. In that series the call to save_nvgprs() prior to > signal handling (do_notify_resume()) is removed, which leaves the > low-bit of regs->trap uncleared which can then trigger the FULL_REGS() > WARNs in setup_tm_sigcontexts(). > > Fixes: 2b0a576d15e0 ("powerpc: Add new transactional memory state to the > signal context") > Cc: sta...@vger.kernel.org # v3.9+ > Signed-off-by: Michael Ellerman <m...@ellerman.id.au>
Applied to powerpc next. https://git.kernel.org/powerpc/c/c7def7fbdeaa25feaa19caf4a27c5d10bd8789e4 cheers