lkml.org 
[lkml]   [2019]   [Oct]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Date
    SubjectRe: [PATCH] Convert filldir[64]() from __put_user() to unsafe_put_user()
    On Mon, Oct 7, 2019 at 11:36 AM Tony Luck <tony.luck@gmail.com> wrote:
    >
    > Late to this party ,,, but my ia64 console today is full of:

    Hmm? I thought ia64 did unaligneds ok.

    But regardless, this is my current (as yet untested) patch. This is
    not the big user access cleanup that I hope Al will do, this is just a
    "ok, x86 is the only one who wants a special unsafe_copy_to_user()
    right now" patch.

    Linus
    arch/x86/include/asm/uaccess.h | 23 ++++++++++++++++++++++
    fs/readdir.c | 44 ++----------------------------------------
    include/linux/uaccess.h | 6 ++++--
    3 files changed, 29 insertions(+), 44 deletions(-)

    diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
    index 35c225ede0e4..61d93f062a36 100644
    --- a/arch/x86/include/asm/uaccess.h
    +++ b/arch/x86/include/asm/uaccess.h
    @@ -734,5 +734,28 @@ do { \
    if (unlikely(__gu_err)) goto err_label; \
    } while (0)

    +/*
    + * We want the unsafe accessors to always be inlined and use
    + * the error labels - thus the macro games.
    + */
    +#define unsafe_copy_loop(dst, src, len, type, label) \
    + while (len >= sizeof(type)) { \
    + unsafe_put_user(*(type *)src,(type __user *)dst,label); \
    + dst += sizeof(type); \
    + src += sizeof(type); \
    + len -= sizeof(type); \
    + }
    +
    +#define unsafe_copy_to_user(_dst,_src,_len,label) \
    +do { \
    + char __user *__ucu_dst = (_dst); \
    + const char *__ucu_src = (_src); \
    + size_t __ucu_len = (_len); \
    + unsafe_copy_loop(__ucu_dst, __ucu_src, __ucu_len, u64, label); \
    + unsafe_copy_loop(__ucu_dst, __ucu_src, __ucu_len, u32, label); \
    + unsafe_copy_loop(__ucu_dst, __ucu_src, __ucu_len, u16, label); \
    + unsafe_copy_loop(__ucu_dst, __ucu_src, __ucu_len, u8, label); \
    +} while (0)
    +
    #endif /* _ASM_X86_UACCESS_H */

    diff --git a/fs/readdir.c b/fs/readdir.c
    index 19bea591c3f1..6e2623e57b2e 100644
    --- a/fs/readdir.c
    +++ b/fs/readdir.c
    @@ -27,53 +27,13 @@
    /*
    * Note the "unsafe_put_user() semantics: we goto a
    * label for errors.
    - *
    - * Also note how we use a "while()" loop here, even though
    - * only the biggest size needs to loop. The compiler (well,
    - * at least gcc) is smart enough to turn the smaller sizes
    - * into just if-statements, and this way we don't need to
    - * care whether 'u64' or 'u32' is the biggest size.
    - */
    -#define unsafe_copy_loop(dst, src, len, type, label) \
    - while (len >= sizeof(type)) { \
    - unsafe_put_user(get_unaligned((type *)src), \
    - (type __user *)dst, label); \
    - dst += sizeof(type); \
    - src += sizeof(type); \
    - len -= sizeof(type); \
    - }
    -
    -/*
    - * We avoid doing 64-bit copies on 32-bit architectures. They
    - * might be better, but the component names are mostly small,
    - * and the 64-bit cases can end up being much more complex and
    - * put much more register pressure on the code, so it's likely
    - * not worth the pain of unaligned accesses etc.
    - *
    - * So limit the copies to "unsigned long" size. I did verify
    - * that at least the x86-32 case is ok without this limiting,
    - * but I worry about random other legacy 32-bit cases that
    - * might not do as well.
    - */
    -#define unsafe_copy_type(dst, src, len, type, label) do { \
    - if (sizeof(type) <= sizeof(unsigned long)) \
    - unsafe_copy_loop(dst, src, len, type, label); \
    -} while (0)
    -
    -/*
    - * Copy the dirent name to user space, and NUL-terminate
    - * it. This should not be a function call, since we're doing
    - * the copy inside a "user_access_begin/end()" section.
    */
    #define unsafe_copy_dirent_name(_dst, _src, _len, label) do { \
    char __user *dst = (_dst); \
    const char *src = (_src); \
    size_t len = (_len); \
    - unsafe_copy_type(dst, src, len, u64, label); \
    - unsafe_copy_type(dst, src, len, u32, label); \
    - unsafe_copy_type(dst, src, len, u16, label); \
    - unsafe_copy_type(dst, src, len, u8, label); \
    - unsafe_put_user(0, dst, label); \
    + unsafe_put_user(0, dst+len, label); \
    + unsafe_copy_to_user(dst, src, len, label); \
    } while (0)


    diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
    index e47d0522a1f4..d4ee6e942562 100644
    --- a/include/linux/uaccess.h
    +++ b/include/linux/uaccess.h
    @@ -355,8 +355,10 @@ extern long strnlen_unsafe_user(const void __user *unsafe_addr, long count);
    #ifndef user_access_begin
    #define user_access_begin(ptr,len) access_ok(ptr, len)
    #define user_access_end() do { } while (0)
    -#define unsafe_get_user(x, ptr, err) do { if (unlikely(__get_user(x, ptr))) goto err; } while (0)
    -#define unsafe_put_user(x, ptr, err) do { if (unlikely(__put_user(x, ptr))) goto err; } while (0)
    +#define unsafe_op_wrap(op, err) do { if (unlikely(op)) goto err; } while (0)
    +#define unsafe_get_user(x,p,e) unsafe_op_wrap(__get_user(x,p),e)
    +#define unsafe_put_user(x,p,e) unsafe_op_wrap(__put_user(x,p),e)
    +#define unsafe_copy_to_user(d,s,l,e) unsafe_op_wrap(__copy_to_user(d,s,l),e)
    static inline unsigned long user_access_save(void) { return 0UL; }
    static inline void user_access_restore(unsigned long flags) { }
    #endif
    \
     
     \ /
      Last update: 2019-10-07 21:09    [W:4.069 / U:0.504 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site