All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Ellerman <patch-notifications@ellerman.id.au>
To: Michael Ellerman <mpe@ellerman.id.au>, linuxppc-dev@ozlabs.org
Cc: mikey@neuling.org, npiggin@gmail.com, gromero@linux.ibm.com
Subject: Re: [PATCH] powerpc/64/tm: Don't let userspace set regs->trap via sigreturn
Date: Wed,  1 Apr 2020 23:53:40 +1100 (AEDT)	[thread overview]
Message-ID: <48smN53KSzz9sTX@ozlabs.org> (raw)
In-Reply-To: <20200401023836.3286664-1-mpe@ellerman.id.au>

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: stable@vger.kernel.org # v3.9+
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>

Applied to powerpc next.

https://git.kernel.org/powerpc/c/c7def7fbdeaa25feaa19caf4a27c5d10bd8789e4

cheers

      reply	other threads:[~2020-04-01 13:57 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-01  2:38 [PATCH] powerpc/64/tm: Don't let userspace set regs->trap via sigreturn Michael Ellerman
2020-04-01 12:53 ` Michael Ellerman [this message]

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=48smN53KSzz9sTX@ozlabs.org \
    --to=patch-notifications@ellerman.id.au \
    --cc=gromero@linux.ibm.com \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=mikey@neuling.org \
    --cc=mpe@ellerman.id.au \
    --cc=npiggin@gmail.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.