[Bug tree-optimization/52798] New: __builtin_object_size() based overflow check is a false positive due to parameter optimisation
gcc at breakpoint dot cc
gcc-bugzilla@gcc.gnu.org
Fri Mar 30 21:54:00 GMT 2012
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52798
Bug #: 52798
Summary: __builtin_object_size() based overflow check is a
false positive due to parameter optimisation
Classification: Unclassified
Product: gcc
Version: 4.7.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: tree-optimization
AssignedTo: unassigned@gcc.gnu.org
ReportedBy: gcc@breakpoint.cc
Host: x86_64-linux-gnu
Target: x86_64-linux-gnu
Build: x86_64-linux-gnu
This is slightly modified from linux kernel fs/binfmt_misc.c:
static int parse_command(const char __user *buffer, size_t count)
{
char s[40];
if (!count)
return 0;
if (count > 3)
return -EINVAL;
if (count == 1 || count == 2 || count == 3)
if (copy_from_user(s, buffer, count))
return -EFAULT;
if (copy_from_user(s, buffer, count))
return -EFAULT;
}
Each copy_from_user() has a __builtin_object_size() based check for length of s
vs length specified in count. The s buffer is 40bytes, we return if count >3.
copy_from_user() produces a #error/#warning for in the overflow case where
count is larger than s.
Since we do check for count >3 and abort, there is no overflow.
gcc on the other hand detects one. Both calls are in my opinion the same. Using
only the first check (count ==1 || count...) avoids the warning, the second
triggers the warning.
The short version of the assembly looks the following way (param via regs in
x86-32, count in edx):
000007e0 <parse_command.part.2>:
8d 72 ff lea -0x1(%edx),%esi
83 fe 02 cmp $0x2,%esi
77 5d ja 858 <parse_command.part.2+0x78>
(esi - 1) compared against 2, so this is the >3 check (and 0 I guess).
#1 call to the copy routine
89 d9 mov %ebx,%ecx
89 fa mov %edi,%edx
e8 fc ff ff ff call 81a <parse_command.part.2+0x3a>
#2 call to the copy routine. There is no check for ebx/ecx in between, so the
count check from #1 is
recognized as unchanged and valid.
This looks to me like gcc knows the exact (and correct) size of s in both cases
but somehow an optimization pass removes it (because it redundant) and the gcc
threats it as not available and therefore produces the warning.
The copy routine looks the following:
static inline unsigned long __must_check copy_from_user(void *to,
const void __user *from,
unsigned long n)
{
int sz = __compiletime_object_size(to);
/* asm volatile(""); */
if (likely(sz == -1 || sz >= n))
n = _copy_from_user(to, from, n);
else
copy_from_user_overflow();
return n;
}
Adding the asm volatile statement (where it is commented out) adds the
follwoing code before the second call:
83 fb 28 cmp $0x28,%ebx
77 65 ja 875 <parse_command.part.2+0x95>
So here the compiler really assumes that there is no upper limit of count.
I attach the .i output of the testcase which can be compiled with:
gcc -O2 -m32 -pipe -c -o binfmt_misc.o binfmt_misc.i
The outout "error: call to ‘co......" is returned if gcc detects the overflow.
Sebastian
More information about the Gcc-bugs
mailing list