lkml.org 
[lkml]   [2020]   [Apr]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.19 125/146] powerpc/64/tm: Dont let userspace set regs->trap via sigreturn
    Date
    From: Michael Ellerman <mpe@ellerman.id.au>

    commit c7def7fbdeaa25feaa19caf4a27c5d10bd8789e4 upstream.

    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>
    Link: https://lore.kernel.org/r/20200401023836.3286664-1-mpe@ellerman.id.au
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    arch/powerpc/kernel/signal_64.c | 4 +++-
    1 file changed, 3 insertions(+), 1 deletion(-)

    --- a/arch/powerpc/kernel/signal_64.c
    +++ b/arch/powerpc/kernel/signal_64.c
    @@ -477,8 +477,10 @@ static long restore_tm_sigcontexts(struc
    err |= __get_user(tsk->thread.ckpt_regs.ccr,
    &sc->gp_regs[PT_CCR]);

    + /* Don't allow userspace to set the trap value */
    + regs->trap = 0;
    +
    /* These regs are not checkpointed; they can go in 'regs'. */
    - err |= __get_user(regs->trap, &sc->gp_regs[PT_TRAP]);
    err |= __get_user(regs->dar, &sc->gp_regs[PT_DAR]);
    err |= __get_user(regs->dsisr, &sc->gp_regs[PT_DSISR]);
    err |= __get_user(regs->result, &sc->gp_regs[PT_RESULT]);

    \
     
     \ /
      Last update: 2020-04-16 18:10    [W:7.105 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site