public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug tree-optimization/52798] New: __builtin_object_size() based overflow check is a false positive due to parameter optimisation
@ 2012-03-30 21:54 gcc at breakpoint dot cc
2012-03-30 23:11 ` [Bug tree-optimization/52798] " gcc at breakpoint dot cc
2012-10-21 22:00 ` dirtyepic at gentoo dot org
0 siblings, 2 replies; 3+ messages in thread
From: gcc at breakpoint dot cc @ 2012-03-30 21:54 UTC (permalink / raw)
To: gcc-bugs
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* [Bug tree-optimization/52798] __builtin_object_size() based overflow check is a false positive due to parameter optimisation
2012-03-30 21:54 [Bug tree-optimization/52798] New: __builtin_object_size() based overflow check is a false positive due to parameter optimisation gcc at breakpoint dot cc
@ 2012-03-30 23:11 ` gcc at breakpoint dot cc
2012-10-21 22:00 ` dirtyepic at gentoo dot org
1 sibling, 0 replies; 3+ messages in thread
From: gcc at breakpoint dot cc @ 2012-03-30 23:11 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52798
--- Comment #1 from Sebastian Andrzej Siewior <gcc at breakpoint dot cc> 2012-03-30 21:54:33 UTC ---
Created attachment 27051
--> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27051
.i test case
^ permalink raw reply [flat|nested] 3+ messages in thread
* [Bug tree-optimization/52798] __builtin_object_size() based overflow check is a false positive due to parameter optimisation
2012-03-30 21:54 [Bug tree-optimization/52798] New: __builtin_object_size() based overflow check is a false positive due to parameter optimisation gcc at breakpoint dot cc
2012-03-30 23:11 ` [Bug tree-optimization/52798] " gcc at breakpoint dot cc
@ 2012-10-21 22:00 ` dirtyepic at gentoo dot org
1 sibling, 0 replies; 3+ messages in thread
From: dirtyepic at gentoo dot org @ 2012-10-21 22:00 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52798
Ryan Hill <dirtyepic at gentoo dot org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |dirtyepic at gentoo dot org
--- Comment #2 from Ryan Hill <dirtyepic at gentoo dot org> 2012-10-21 22:00:24 UTC ---
Also 4.6.2, 4.6.3.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-10-21 22:00 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-30 21:54 [Bug tree-optimization/52798] New: __builtin_object_size() based overflow check is a false positive due to parameter optimisation gcc at breakpoint dot cc
2012-03-30 23:11 ` [Bug tree-optimization/52798] " gcc at breakpoint dot cc
2012-10-21 22:00 ` dirtyepic at gentoo dot org
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).