lkml.org 
[lkml]   [2011]   [Jul]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[RFC v3 5/5] x86: implement RUNTIME_USER_COPY_CHECK
    Date
    Introduce kernel_access_ok() checks in {,__}copy_{to,from}_user().
    Implement x86 variant of arch_check_object_on_stack_frame(), which
    checks whether the copied buffer doesn't overflow a single stack frame.
    The boundaries of the frame are the saved return address and the saved
    EBP/RBP register. This information is available in case of
    CONFIG_FRAME_POINTERS=y only, otherwise don't check anything and simply
    return true.

    v3 - Used __always_inline.
    - Now log the copy direction (from/to user) on overflows.
    - Used #ifdef instead of #if defined().

    Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
    ---
    arch/x86/include/asm/uaccess.h | 49 +++++++++++++++++++++++++++++++++++++
    arch/x86/include/asm/uaccess_32.h | 32 ++++++++++++++++++++++--
    arch/x86/include/asm/uaccess_64.h | 38 +++++++++++++++++++++++++++-
    arch/x86/lib/usercopy_32.c | 2 +-
    4 files changed, 115 insertions(+), 6 deletions(-)

    diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
    index 99ddd14..f81a1d6 100644
    --- a/arch/x86/include/asm/uaccess.h
    +++ b/arch/x86/include/asm/uaccess.h
    @@ -9,6 +9,7 @@
    #include <linux/string.h>
    #include <asm/asm.h>
    #include <asm/page.h>
    +#include <linux/uaccess-check.h>

    #define VERIFY_READ 0
    #define VERIFY_WRITE 1
    @@ -78,6 +79,54 @@
    */
    #define access_ok(type, addr, size) (likely(__range_not_ok(addr, size) == 0))

    +#ifdef CONFIG_FRAME_POINTER
    +/*
    + * MUST be always_inline to correctly count stack frame numbers.
    + *
    + * low ----------------------------------------------> high
    + * [saved bp][saved ip][args][local vars][saved bp][saved ip]
    + * ^----------------^
    + * allow copies only within here
    +*/
    +#undef arch_check_object_on_stack_frame
    +static __always_inline bool arch_check_object_on_stack_frame(const void *stack,
    + const void *stackend, const void *obj, unsigned long len)
    +{
    + const void *frame = NULL;
    + const void *oldframe;
    +
    + /*
    + * Get the kernel_access_ok() caller frame.
    + * __builtin_frame_address(0) returns __kernel_access_ok() frame
    + * as arch_ and stack_ are inline and __kernel_ is noinline.
    + */
    + oldframe = __builtin_frame_address(0);
    + if (oldframe)
    + frame = __builtin_frame_address(1);
    +
    + while (stack <= frame && frame < stackend) {
    + /*
    + * If obj + len extends past the last frame, this
    + * check won't pass and the next frame will be 0,
    + * causing us to bail out and correctly report
    + * the copy as invalid.
    + */
    + if (obj + len <= frame) {
    + /* EBP + EIP (or RBP + RIP for x86-64) */
    + size_t protected_regs_size = 2*sizeof(void *);
    +
    + if (obj >= oldframe + protected_regs_size)
    + return true;
    + return false;
    + }
    + oldframe = frame;
    + frame = *(const void * const *)frame;
    + }
    + return false;
    +}
    +#define arch_check_object_on_stack_frame arch_check_object_on_stack_frame
    +#endif /* CONFIG_FRAME_POINTER */
    +
    /*
    * The exception table consists of pairs of addresses: the first is the
    * address of an instruction that is allowed to fault, and the second is
    diff --git a/arch/x86/include/asm/uaccess_32.h b/arch/x86/include/asm/uaccess_32.h
    index 566e803..669a2b6 100644
    --- a/arch/x86/include/asm/uaccess_32.h
    +++ b/arch/x86/include/asm/uaccess_32.h
    @@ -82,6 +82,8 @@ static __always_inline unsigned long __must_check
    __copy_to_user(void __user *to, const void *from, unsigned long n)
    {
    might_fault();
    + if (!kernel_access_ok(from, n, true))
    + return n;
    return __copy_to_user_inatomic(to, from, n);
    }

    @@ -152,6 +154,8 @@ __copy_from_user(void *to, const void __user *from, unsigned long n)
    return ret;
    }
    }
    + if (!kernel_access_ok(to, n, false))
    + return n;
    return __copy_from_user_ll(to, from, n);
    }

    @@ -205,13 +209,35 @@ static inline unsigned long __must_check copy_from_user(void *to,
    {
    int sz = __compiletime_object_size(to);

    - if (likely(sz == -1 || sz >= n))
    - n = _copy_from_user(to, from, n);
    - else
    + if (likely(sz == -1 || sz >= n)) {
    + if (kernel_access_ok(to, n, false))
    + n = _copy_from_user(to, from, n);
    + } else {
    copy_from_user_overflow();
    + }
    +
    + return n;
    +}

    +#undef copy_from_user_unchecked
    +static inline unsigned long __must_check copy_from_user_unchecked(void *to,
    + const void __user *from,
    + unsigned long n)
    +{
    + return _copy_from_user(to, from, n);
    +}
    +#define copy_from_user_unchecked copy_from_user_unchecked
    +
    +#undef copy_to_user_unchecked
    +static inline unsigned long copy_to_user_unchecked(void __user *to,
    + const void *from,
    + unsigned long n)
    +{
    + if (access_ok(VERIFY_WRITE, to, n))
    + n = __copy_to_user(to, from, n);
    return n;
    }
    +#define copy_to_user_unchecked copy_to_user_unchecked

    long __must_check strncpy_from_user(char *dst, const char __user *src,
    long count);
    diff --git a/arch/x86/include/asm/uaccess_64.h b/arch/x86/include/asm/uaccess_64.h
    index 1c66d30..407facb 100644
    --- a/arch/x86/include/asm/uaccess_64.h
    +++ b/arch/x86/include/asm/uaccess_64.h
    @@ -50,8 +50,10 @@ static inline unsigned long __must_check copy_from_user(void *to,
    int sz = __compiletime_object_size(to);

    might_fault();
    - if (likely(sz == -1 || sz >= n))
    - n = _copy_from_user(to, from, n);
    + if (likely(sz == -1 || sz >= n)) {
    + if (kernel_access_ok(to, n, false))
    + n = _copy_from_user(to, from, n);
    + }
    #ifdef CONFIG_DEBUG_VM
    else
    WARN(1, "Buffer overflow detected!\n");
    @@ -59,13 +61,37 @@ static inline unsigned long __must_check copy_from_user(void *to,
    return n;
    }

    +#undef copy_from_user_unchecked
    +static inline unsigned long __must_check copy_from_user_unchecked(void *to,
    + const void __user *from,
    + unsigned long n)
    +{
    + might_fault();
    +
    + return _copy_from_user(to, from, n);
    +}
    +#define copy_from_user_unchecked copy_from_user_unchecked
    +
    static __always_inline __must_check
    int copy_to_user(void __user *dst, const void *src, unsigned size)
    {
    might_fault();

    + if (!kernel_access_ok(src, size, true))
    + return size;
    +
    + return _copy_to_user(dst, src, size);
    +}
    +
    +#undef copy_to_user_unchecked
    +static __always_inline __must_check
    +int copy_to_user_unchecked(void __user *dst, const void *src, unsigned size)
    +{
    + might_fault();
    +
    return _copy_to_user(dst, src, size);
    }
    +#define copy_to_user_unchecked copy_to_user_unchecked

    static __always_inline __must_check
    int __copy_from_user(void *dst, const void __user *src, unsigned size)
    @@ -73,8 +99,12 @@ int __copy_from_user(void *dst, const void __user *src, unsigned size)
    int ret = 0;

    might_fault();
    + if (!kernel_access_ok(dst, size, false))
    + return size;
    +
    if (!__builtin_constant_p(size))
    return copy_user_generic(dst, (__force void *)src, size);
    +
    switch (size) {
    case 1:__get_user_asm(*(u8 *)dst, (u8 __user *)src,
    ret, "b", "b", "=q", 1);
    @@ -117,8 +147,12 @@ int __copy_to_user(void __user *dst, const void *src, unsigned size)
    int ret = 0;

    might_fault();
    + if (!kernel_access_ok(dst, size, true))
    + return size;
    +
    if (!__builtin_constant_p(size))
    return copy_user_generic((__force void *)dst, src, size);
    +
    switch (size) {
    case 1:__put_user_asm(*(u8 *)src, (u8 __user *)dst,
    ret, "b", "b", "iq", 1);
    diff --git a/arch/x86/lib/usercopy_32.c b/arch/x86/lib/usercopy_32.c
    index e218d5d..4c8b5b5 100644
    --- a/arch/x86/lib/usercopy_32.c
    +++ b/arch/x86/lib/usercopy_32.c
    @@ -851,7 +851,7 @@ EXPORT_SYMBOL(__copy_from_user_ll_nocache_nozero);
    unsigned long
    copy_to_user(void __user *to, const void *from, unsigned long n)
    {
    - if (access_ok(VERIFY_WRITE, to, n))
    + if (access_ok(VERIFY_WRITE, to, n) && kernel_access_ok(from, n, true))
    n = __copy_to_user(to, from, n);
    return n;
    }
    --

    \
     
     \ /
      Last update: 2011-07-21 14:57    [W:0.035 / U:60.908 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site