• Michael Ellerman's avatar
    powerpc: Use asm_goto_volatile for put_user() · 1344a232
    Michael Ellerman authored
    Andreas reported that commit ee0a49a6 ("powerpc/uaccess: Switch
    __put_user_size_allowed() to __put_user_asm_goto()") broke
    CLONE_CHILD_SETTID.
    
    Further inspection showed that the put_user() in schedule_tail() was
    missing entirely, the store not emitted by the compiler.
    
      <.schedule_tail>:
        mflr    r0
        std     r0,16(r1)
        stdu    r1,-112(r1)
        bl      <.finish_task_switch>
        ld      r9,2496(r3)
        cmpdi   cr7,r9,0
        bne     cr7,<.schedule_tail+0x60>
        ld      r3,392(r13)
        ld      r9,1392(r3)
        cmpdi   cr7,r9,0
        beq     cr7,<.schedule_tail+0x3c>
        li      r4,0
        li      r5,0
        bl      <.__task_pid_nr_ns>
        nop
        bl      <.calculate_sigpending>
        nop
        addi    r1,r1,112
        ld      r0,16(r1)
        mtlr    r0
        blr
        nop
        nop
        nop
        bl      <.__balance_callback>
        b       <.schedule_tail+0x1c>
    
    Notice there are no stores other than to the stack. There should be a
    stw in there for the store to current->set_child_tid.
    
    This is only seen with GCC 4.9 era compilers (tested with 4.9.3 and
    4.9.4), and only when CONFIG_PPC_KUAP is disabled.
    
    When CONFIG_PPC_KUAP=y, the inline asm that's part of the isync()
    and mtspr() inlined via allow_user_access() seems to be enough to
    avoid the bug.
    
    We already have a macro to work around this (or a similar bug), called
    asm_volatile_goto which includes an empty asm block to tickle the
    compiler into generating the right code. So use that.
    
    With this applied the code generation looks more like it will work:
    
      <.schedule_tail>:
        mflr    r0
        std     r31,-8(r1)
        std     r0,16(r1)
        stdu    r1,-144(r1)
        std     r3,112(r1)
        bl      <._mcount>
        nop
        ld      r3,112(r1)
        bl      <.finish_task_switch>
        ld      r9,2624(r3)
        cmpdi   cr7,r9,0
        bne     cr7,<.schedule_tail+0xa0>
        ld      r3,2408(r13)
        ld      r31,1856(r3)
        cmpdi   cr7,r31,0
        beq     cr7,<.schedule_tail+0x80>
        li      r4,0
        li      r5,0
        bl      <.__task_pid_nr_ns>
        nop
        li      r9,-1
        clrldi  r9,r9,12
        cmpld   cr7,r31,r9
        bgt     cr7,<.schedule_tail+0x80>
        lis     r9,16
        rldicr  r9,r9,32,31
        subf    r9,r31,r9
        cmpldi  cr7,r9,3
        ble     cr7,<.schedule_tail+0x80>
        li      r9,0
        stw     r3,0(r31)				<-- stw
        nop
        bl      <.calculate_sigpending>
        nop
        addi    r1,r1,144
        ld      r0,16(r1)
        ld      r31,-8(r1)
        mtlr    r0
        blr
        nop
        bl      <.__balance_callback>
        b       <.schedule_tail+0x30>
    
    Fixes: ee0a49a6 ("powerpc/uaccess: Switch __put_user_size_allowed() to __put_user_asm_goto()")
    Reported-by: default avatarAndreas Schwab <schwab@linux-m68k.org>
    Tested-by: default avatarAndreas Schwab <schwab@linux-m68k.org>
    Suggested-by: default avatarChristophe Leroy <christophe.leroy@csgroup.eu>
    Signed-off-by: default avatarMichael Ellerman <mpe@ellerman.id.au>
    Link: https://lore.kernel.org/r/20201104111742.672142-1-mpe@ellerman.id.au
    1344a232
uaccess.h 16.2 KB