| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <alpine.LFD.2.01.0907121106530.3552@localhost.localdomain>
Date: Sun, 12 Jul 2009 11:24:03 -0700 (PDT)
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Frans Pop <elendil@...net.nl>
cc: Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Andrew Morton <akpm@...ux-foundation.org>,
linux-kbuild@...r.kernel.org, barryn@...ox.com,
bugme-daemon@...zilla.kernel.org,
Ian Lance Taylor <iant@...gle.com>
Subject: Re: [Bug 13012] 2.6.28.9 causes init to segfault on Debian etch;
2.6.28.8 OK
On Sun, 12 Jul 2009, Linus Torvalds wrote:
>
> From everything I have been able to find, I really prefer the second
> version. Not only is the patch cleaner, but it looks like code generation
> is better too (for some inexplicable reason, but I suspect it's because
> -fno-strict-overflow is just saner).
Hmm. I just checked. The file that caused us to do this thing in the first
place (fs/open.c, around like 415, which does:
/* Check for wrap through zero too */
if (((offset + len) > inode->i_sb->s_maxbytes) || ((offset + len) < 0))
goto out_fput;
to check that the resulting 'loffset_t' type is all good) has interesting
behaviour with my version of gcc (gcc version 4.4.0 20090506 (Red Hat
4.4.0-4) (GCC)).
- Without any options:
leaq (%rcx,%rdx), %rdi #, tmp73
movq 256(%rbx), %rsi # <variable>.i_sb, <variable>.i_sb
movl $-27, %eax #, D.29131
cmpq 40(%rsi), %rdi # <variable>.s_maxbytes, tmp73
ja .L148 #,
- With -fno-strict-overflow:
leaq (%rcx,%rdx), %rax #, D.29157
movq 256(%rbx), %rsi # <variable>.i_sb, <variable>.i_sb
cmpq 40(%rsi), %rax # <variable>.s_maxbytes, D.29157
ja .L154 #,
testq %rax, %rax # D.29157
js .L154 #,
- With -fwrapv:
leaq (%rcx,%rdx), %rax #, D.29158
movq 256(%rbx), %rsi # <variable>.i_sb, <variable>.i_sb
cmpq 40(%rsi), %rax # <variable>.s_maxbytes, D.29158
ja .L154 #,
testq %rax, %rax # D.29158
js .L154 #,
and from this it would look like:
- gcc on its own is actually the best version (the first comparison is
unsigned because s_maxbytes is actually 'unsigned long long', so it
actually does the right thing!)
In other words, the whole '< 0' was unnecessary, but does make the
source code way more readable, and makes the source code _correct_
regardless of any type issues!
- From a cursory inspection, -fno-strict-overflow and -fwrapv are both
equivalent in this code, and both do the stupid thing (but for good
reason - gcc doesn't know that 's_maxbytes' might not be 'negative in a
loffset_t', so technically speaking the extraneous 'js' is not
extraneous, because it can actually trigger some "more negative"
entries than s_maxbyes is.
- HOWEVER:
[torvalds@...alem ~]$ git diff --stat open.s open.s-fno-strict-overflow
open.s => open.s-fno-strict-overflow | 22 +++++++++++++---------
1 files changed, 13 insertions(+), 9 deletions(-)
[torvalds@...alem ~]$ git diff --stat open.s open.s-fwrapv
open.s => open.s-fwrapv | 296 ++++++++++++++++++++++++-----------------------
1 files changed, 150 insertions(+), 146 deletions(-)
where the _only_ difference that '-fno-strict-overflow' introduces is
that one small area (it's saying 22 lines changed, but that's because
there's also the compiler option listing at the top of the file etc)
In contrast, -fwrapv has done a lot of other changes too. Now, in both
cases, it really only added the same four instructions (testq + js +
branchtarget + jumparound).
It looks like 'fwrapv' generates more temporaries (possibly for the code
that treies to enforce the exact twos-complement behavior) that then all
get optimized back out again. The differences seem to be in the temporary
variable numbers etc, not in the actual code.
So fwrapv really _is_ different from fno-strict-pverflow, and disturbs the
code generation more.
IOW, I'm convinced we should never use fwrapv. It's clearly a buggy piece
of sh*t, as shown by our 4.1.x experiences. We should use
-fno-strict-overflow.
Will commit the following (which also fits naming-wise with our use of
'-fno-strict-aliasing').
Linus
---
Makefile | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/Makefile b/Makefile
index 0aeec59..bbe8453 100644
--- a/Makefile
+++ b/Makefile
@@ -565,7 +565,7 @@ KBUILD_CFLAGS += $(call cc-option,-Wdeclaration-after-statement,)
KBUILD_CFLAGS += $(call cc-option,-Wno-pointer-sign,)
# disable invalid "can't wrap" optimizations for signed / pointers
-KBUILD_CFLAGS += $(call cc-option,-fwrapv)
+KBUILD_CFLAGS += $(call cc-option,-fno-strict-overflow)
# revert to pre-gcc-4.4 behaviour of .eh_frame
KBUILD_CFLAGS += $(call cc-option,-fno-dwarf2-cfi-asm)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists