Skip to content

Commit

Permalink
x86: Turn the copy_from_user check into an (optional) compile time wa…
Browse files Browse the repository at this point in the history
…rning

A previous patch added the buffer size check to copy_from_user().

One of the things learned from analyzing the result of the previous
patch is that in general, gcc is really good at proving that the
code contains sufficient security checks to not need to do a
runtime check. But that for those cases where gcc could not prove
this, there was a relatively high percentage of real security
issues.

This patch turns the case of "gcc cannot prove" into a compile time
warning, as long as a sufficiently new gcc is in use that supports
this. The objective is that these warnings will trigger developers
checking new cases out before a security hole enters a linux kernel
release.

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: James Morris <jmorris@namei.org>
Cc: Jan Beulich <jbeulich@novell.com>
LKML-Reference: <20090930130523.348ae6c4@infradead.org>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
  • Loading branch information
Arjan van de Ven authored and Ingo Molnar committed Oct 1, 2009
1 parent ff60fab commit 4a31276
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 3 deletions.
12 changes: 9 additions & 3 deletions arch/x86/include/asm/uaccess_32.h
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,13 @@ unsigned long __must_check _copy_from_user(void *to,
const void __user *from,
unsigned long n);


extern void copy_from_user_overflow(void)
#ifdef CONFIG_DEBUG_STACKOVERFLOW
__compiletime_warning("copy_from_user() buffer size is not provably correct")
#endif
;

static inline unsigned long __must_check copy_from_user(void *to,
const void __user *from,
unsigned long n)
Expand All @@ -200,10 +207,9 @@ static inline unsigned long __must_check copy_from_user(void *to,

if (likely(sz == -1 || sz >= n))
ret = _copy_from_user(to, from, n);
#ifdef CONFIG_DEBUG_VM
else
WARN(1, "Buffer overflow detected!\n");
#endif
copy_from_user_overflow();

return ret;
}

Expand Down
6 changes: 6 additions & 0 deletions arch/x86/lib/usercopy_32.c
Original file line number Diff line number Diff line change
Expand Up @@ -883,3 +883,9 @@ _copy_from_user(void *to, const void __user *from, unsigned long n)
return n;
}
EXPORT_SYMBOL(_copy_from_user);

void copy_from_user_overflow(void)
{
WARN(1, "Buffer overflow detected!\n");
}
EXPORT_SYMBOL(copy_from_user_overflow);
3 changes: 3 additions & 0 deletions include/linux/compiler-gcc4.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,6 @@
#endif

#define __compiletime_object_size(obj) __builtin_object_size(obj, 0)
#if __GNUC_MINOR__ >= 4
#define __compiletime_warning(message) __attribute__((warning(message)))
#endif
4 changes: 4 additions & 0 deletions include/linux/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,10 @@ void ftrace_likely_update(struct ftrace_branch_data *f, int val, int expect);
#ifndef __compiletime_object_size
# define __compiletime_object_size(obj) -1
#endif
#ifndef __compiletime_warning
# define __compiletime_warning(message)
#endif

/*
* Prevent the compiler from merging or refetching accesses. The compiler
* is also forbidden from reordering successive instances of ACCESS_ONCE(),
Expand Down

0 comments on commit 4a31276

Please sign in to comment.