[lkml]   [2010]   [Aug]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    Patch in this message
    Subject[PATCH v2] ARM: uaccess: Implement strict user copy checks
    Programmers can easily forget to ensure their buffer size is
    large enough to receive all the user data when calling
    copy_from_user(). Add some sanity checking to copy_from_user() by
    using the builtin __builtin_object_size() provided by newer GCC's
    (4.4 and greater) to prove at compile time that the kernel buffer
    won't overflow. This should catch some security holes earlier
    since at compile time we'll either see a warning stating that
    GCC can't prove the buffer size is large enough, or an error to
    the same effect if CONFIG_DEBUG_STRICT_USER_COPY_CHECKS=y.

    These checks are optimized away when GCC can't prove the buffer
    will overflow and when it can prove the buffer won't overflow.

    This patch is inspired by 9f0cf4a (x86: Use
    __builtin_object_size() to validate the buffer size for
    copy_from_user(), 2009-09-26).

    Signed-off-by: Stephen Boyd <>
    Cc: Arnd Bergmann <>
    Cc: Russell King <>


    I'm unsure what needs to be done for the follow up patch. Shouldn't
    it be multiple patches sent to each arch maintainer to fix the

    * More descriptive commit text
    * dependency on !TRACE_BRANCH_PROFILING (see ad8f435 (x86: Don't use
    the strict copy checks when branch profiling is in use, 2009-10-06))

    arch/arm/Kconfig.debug | 14 ++++++++++++++
    arch/arm/include/asm/uaccess.h | 14 ++++++++++++++
    arch/arm/lib/Makefile | 3 ++-
    arch/arm/lib/usercopy.c | 25 +++++++++++++++++++++++++
    4 files changed, 55 insertions(+), 1 deletions(-)
    create mode 100644 arch/arm/lib/usercopy.c

    diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
    index 91344af..64e33b8 100644
    --- a/arch/arm/Kconfig.debug
    +++ b/arch/arm/Kconfig.debug
    @@ -128,4 +128,18 @@ config DEBUG_S3C_UART
    The uncompressor code port configuration is now handled

    + bool "Strict user copy size checks"
    + help
    + Enabling this option turns a certain set of sanity checks for user
    + copy operations into compile time errors.
    + The copy_from_user() etc checks are there to help test if there
    + are sufficient security checks on the length argument of
    + the copy operation, by having gcc prove that the argument is
    + within bounds.
    + If unsure, or if you run an older (pre 4.4) gcc, say N.
    diff --git a/arch/arm/include/asm/uaccess.h b/arch/arm/include/asm/uaccess.h
    index 33e4a48..3153e1a 100644
    --- a/arch/arm/include/asm/uaccess.h
    +++ b/arch/arm/include/asm/uaccess.h
    @@ -401,8 +401,22 @@ extern unsigned long __must_check __clear_user_std(void __user *addr, unsigned l
    extern unsigned long __must_check __strncpy_from_user(char *to, const char __user *from, unsigned long count);
    extern unsigned long __must_check __strnlen_user(const char __user *s, long n);

    +extern void copy_from_user_overflow(void)
    + __compiletime_error("copy_from_user() buffer size is not provably correct")
    + __compiletime_warning("copy_from_user() buffer size is not provably correct")
    static inline unsigned long __must_check copy_from_user(void *to, const void __user *from, unsigned long n)
    + unsigned int sz = __compiletime_object_size(to);
    + if (unlikely(sz != -1 && sz < n)) {
    + copy_from_user_overflow();
    + return n;
    + }
    if (access_ok(VERIFY_READ, from, n))
    n = __copy_from_user(to, from, n);
    else /* security hole - plug it */
    diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile
    index 59ff42d..561cf3d 100644
    --- a/arch/arm/lib/Makefile
    +++ b/arch/arm/lib/Makefile
    @@ -13,7 +13,8 @@ lib-y := backtrace.o changebit.o csumipv6.o csumpartial.o \
    testchangebit.o testclearbit.o testsetbit.o \
    ashldi3.o ashrdi3.o lshrdi3.o muldi3.o \
    ucmpdi2.o lib1funcs.o div64.o sha1.o \
    - io-readsb.o io-writesb.o io-readsl.o io-writesl.o
    + io-readsb.o io-writesb.o io-readsl.o io-writesl.o \
    + usercopy.o

    mmu-y := clear_user.o copy_page.o getuser.o putuser.o

    diff --git a/arch/arm/lib/usercopy.c b/arch/arm/lib/usercopy.c
    new file mode 100644
    index 0000000..e57e6e2
    --- /dev/null
    +++ b/arch/arm/lib/usercopy.c
    @@ -0,0 +1,25 @@
    + * Copyright (c) 2009-2010, Code Aurora Forum. All rights reserved.
    + *
    + * This program is free software; you can redistribute it and/or modify
    + * it under the terms of the GNU General Public License version 2 and
    + * only version 2 as published by the Free Software Foundation.
    + *
    + * This program is distributed in the hope that it will be useful,
    + * but WITHOUT ANY WARRANTY; without even the implied warranty of
    + * GNU General Public License for more details.
    + *
    + * You should have received a copy of the GNU General Public License
    + * along with this program; if not, write to the Free Software
    + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
    + * 02110-1301, USA.
    + */
    +#include <linux/module.h>
    +#include <linux/bug.h>
    +void copy_from_user_overflow(void)
    + WARN(1, "Buffer overflow detected!\n");
    Sent by an employee of the Qualcomm Innovation Center, Inc.
    The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

     \ /
      Last update: 2010-08-18 03:31    [W:0.025 / U:5.940 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site