• Ben Dooks's avatar
    riscv: evaluate put_user() arg before enabling user access · 285a76bb
    Ben Dooks authored
    The <asm/uaccess.h> header has a problem with put_user(a, ptr) if
    the 'a' is not a simple variable, such as a function. This can lead
    to the compiler producing code as so:
    
    1:	enable_user_access()
    2:	evaluate 'a' into register 'r'
    3:	put 'r' to 'ptr'
    4:	disable_user_acess()
    
    The issue is that 'a' is now being evaluated with the user memory
    protections disabled. So we try and force the evaulation by assigning
    'x' to __val at the start, and hoping the compiler barriers in
     enable_user_access() do the job of ordering step 2 before step 1.
    
    This has shown up in a bug where 'a' sleeps and thus schedules out
    and loses the SR_SUM flag. This isn't sufficient to fully fix, but
    should reduce the window of opportunity. The first instance of this
    we found is in scheudle_tail() where the code does:
    
    $ less -N kernel/sched/core.c
    
    4263  if (current->set_child_tid)
    4264         put_user(task_pid_vnr(current), current->set_child_tid);
    
    Here, the task_pid_vnr(current) is called within the block that has
    enabled the user memory access. This can be made worse with KASAN
    which makes task_pid_vnr() a rather large call with plenty of
    opportunity to sleep.
    Signed-off-by: default avatarBen Dooks <ben.dooks@codethink.co.uk>
    Reported-by: syzbot+e74b94fe601ab9552d69@syzkaller.appspotmail.com
    Suggested-by: default avatarArnd Bergman <arnd@arndb.de>
    
    --
    Changes since v1:
    - fixed formatting and updated the patch description with more info
    
    Changes since v2:
    - fixed commenting on __put_user() (schwab@linux-m68k.org)
    
    Change since v3:
    - fixed RFC in patch title. Should be ready to merge.
    Signed-off-by: default avatarPalmer Dabbelt <palmerdabbelt@google.com>
    285a76bb
uaccess.h 13.7 KB