07 Aug, 2014
2 commits
-
We have been chasing a memory corruption bug, which turned out to be
caused by very old gcc (4.3.4), which happily turned conditional load
into a non-conditional one, and that broke correctness (the condition
was met only if lock was held) and corrupted memory.This particular problem with that particular code did not happen when
never gccs were used. I've brought this up with our gcc folks, as I
wanted to make sure that this can't really happen again, and it turns
out it actually can.Quoting Martin Jambor :
"More current GCCs are more careful when it comes to replacing a
conditional load with a non-conditional one, most notably they check
that a store happens in each iteration of _a_ loop but they assume
loops are executed. They also perform a simple check whether the
store cannot trap which currently passes only for non-const
variables. A simple testcase demonstrating it on an x86_64 is for
example the following:$ cat cond_store.c
int g_1 = 1;
int g_2[1024] __attribute__((section ("safe_section"), aligned (4096)));
int c = 4;
int __attribute__ ((noinline))
foo (void)
{
int l;
for (l = 0; (l != 4); l++) {
if (g_1)
return l;
for (g_2[0] = 0; (g_2[0] >= 26); ++g_2[0])
;
}
return 2;
}int main (int argc, char* argv[])
{
if (mprotect (g_2, sizeof(g_2), PROT_READ) == -1)
{
int e = errno;
error (e, e, "mprotect error %i", e);
}
foo ();
__builtin_printf("OK\n");
return 0;
}
/* EOF */
$ ~/gcc/trunk/inst/bin/gcc cond_store.c -O2 --param allow-store-data-races=0
$ ./a.out
OK
$ ~/gcc/trunk/inst/bin/gcc cond_store.c -O2 --param allow-store-data-races=1
$ ./a.out
Segmentation faultThe testcase fails the same at least with 4.9, 4.8 and 4.7. Therefore
I would suggest building kernels with this parameter set to zero. I
also agree with Jikos that the default should be changed for -O2. I
have run most of the SPEC 2k6 CPU benchmarks (gamess and dealII
failed, at -O2, not sure why) compiled with and without this option
and did not see any real difference between respective run-times"Hopefully the default will be changed in newer gccs, but let's force it
for kernel builds so that we are on a safe side even when older gcc are
used.The code in question was out-of-tree printk-in-NMI (yeah, surprise
suprise, once again) patch written by Petr Mladek, let me quote his
comment from our internal bugzilla:"I have spent few days investigating inconsistent state of kernel ring buffer.
It went out that it was caused by speculative store generated by
gcc-4.3.4.The problem is in assembly generated for make_free_space(). The functions is
called the following way:+ vprintk_emit();
+ log = MAIN_LOG; // with logbuf_lock
or
log = NMI_LOG; // with nmi_logbuf_lock
cont_add(log, ...);
+ cont_flush(log, ...);
+ log_store(log, ...);
+ log_make_free_space(log, ...);If called with log = NMI_LOG then only nmi_log_* global variables are safe to
modify but the generated code does store also into (main_)log_* global
variables::
55 push %rbp
89 f6 mov %esi,%esi48 8b 05 03 99 51 01 mov 0x1519903(%rip),%rax # ffffffff82620868
44 8b 1d ec 98 51 01 mov 0x15198ec(%rip),%r11d # ffffffff82620858
8b 35 36 60 14 01 mov 0x1146036(%rip),%esi # ffffffff8224cfa8
44 8b 35 33 60 14 01 mov 0x1146033(%rip),%r14d # ffffffff8224cfac
4c 8b 2d d0 98 51 01 mov 0x15198d0(%rip),%r13 # ffffffff82620850
4c 8b 25 11 61 14 01 mov 0x1146111(%rip),%r12 # ffffffff8224d098
49 89 c2 mov %rax,%r10
48 21 c2 and %rax,%rdx
48 8b 1d 0c 99 55 01 mov 0x155990c(%rip),%rbx # ffffffff826608a0
49 c1 ea 20 shr $0x20,%r10
48 89 55 d0 mov %rdx,-0x30(%rbp)
44 29 de sub %r11d,%esi
45 29 d6 sub %r10d,%r14d
4c 8b 0d 97 98 51 01 mov 0x1519897(%rip),%r9 # ffffffff82620840
eb 7e jmp ffffffff81107029
[...]
85 ff test %edi,%edi # edi = 1 for NMI_LOG
4c 89 e8 mov %r13,%rax
4c 89 ca mov %r9,%rdx
74 0a je ffffffff8110703d
8b 15 27 98 51 01 mov 0x1519827(%rip),%edx # ffffffff82620860
48 8b 45 d0 mov -0x30(%rbp),%rax
48 39 c2 cmp %rax,%rdx # end of loop
0f 84 da 00 00 00 je ffffffff81107120
[...]
85 ff test %edi,%edi # edi = 1 for NMI_LOG
4c 89 0d 17 97 51 01 mov %r9,0x1519717(%rip) # ffffffff82620840
^^^^^^^^^^^^^^^^^^^^^^^^^^
KABOOOM
74 35 je ffffffff81107160It stores log_first_seq when edi == NMI_LOG. This instructions are used also
when edi == MAIN_LOG but the store is done speculatively before the condition
is decided. It is unsafe because we do not have "logbuf_lock" in NMI context
and some other process migh modify "log_first_seq" in parallel"I believe that the best course of action is both
- building kernel (and anything multi-threaded, I guess) with that
optimization turned off
- persuade gcc folks to change the default for future releasesSigned-off-by: Jiri Kosina
Cc: Martin Jambor
Cc: Petr Mladek
Cc: Linus Torvalds
Cc: Paul E. McKenney
Cc: Peter Zijlstra
Cc: Marek Polacek
Cc: Jakub Jelinek
Cc: Steven Noonan
Cc: Richard Biener
Cc: Dan Carpenter
Signed-off-by: Andrew Morton
Signed-off-by: Linus Torvalds -
This adds a hopefully helpful comment above the (seemingly weird) compiler
flag selection logic.Signed-off-by: Kees Cook
Suggested-by: Andrew Morton
Cc: Andi Kleen
Cc: Randy Dunlap
Cc: Michal Marek
Cc: Michal Hocko
Cc: Stephen Rothwell
Signed-off-by: Andrew Morton
Signed-off-by: Linus Torvalds
04 Aug, 2014
1 commit
28 Jul, 2014
1 commit
27 Jul, 2014
1 commit
-
Michel Dänzer and a couple of other people reported inexplicable random
oopses in the scheduler, and the cause turns out to be gcc mis-compiling
the load_balance() function when debugging is enabled. The gcc bug
apparently goes back to gcc-4.5, but slight optimization changes means
that it now showed up as a problem in 4.9.0 and 4.9.1.The instruction scheduling problem causes gcc to schedule a spill
operation to before the stack frame has been created, which in turn can
corrupt the spilled value if an interrupt comes in. There may be other
effects of this bug too, but that's the code generation problem seen in
Michel's case.This is fixed in current gcc HEAD, but the workaround as suggested by
Markus Trippelsdorf is pretty simple: use -fno-var-tracking-assignments
when compiling the kernel, which disables the gcc code that causes the
problem. This can result in slightly worse debug information for
variable accesses, but that is infinitely preferable to actual code
generation problems.Doing this unconditionally (not just for CONFIG_DEBUG_INFO) also allows
non-debug builds to verify that the debug build would be identical: we
can doexport GCC_COMPARE_DEBUG=1
to make gcc internally verify that the result of the build is
independent of the "-g" flag (it will make the compiler build everything
twice, toggling the debug flag, and compare the results).Without the "-fno-var-tracking-assignments" option, the build would fail
(even with 4.8.3 that didn't show the actual stack frame bug) with a gcc
compare failure.See also gcc bugzilla:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61801
Reported-by: Michel Dänzer
Suggested-by: Markus Trippelsdorf
Cc: Jakub Jelinek
Cc: stable@kernel.org
Signed-off-by: Linus Torvalds
21 Jul, 2014
1 commit
14 Jul, 2014
1 commit
11 Jul, 2014
1 commit
-
Pull kbuild fixes from Michal Marek:
"Three more fixes for the relative build dir feature:- Shut up make -s again
- Fix for rpm/deb/tar-pkg with O=
- Fix for CONFIG_EXTRA_FIRMWARE"* 'rc-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/mmarek/kbuild:
firmware: Create directories for external firmware
kbuild: Fix packaging targets with relative $(srctree)
kbuild: Do not print the build directory with make -s
07 Jul, 2014
1 commit
05 Jul, 2014
1 commit
-
All other users of Makefile.build set $(obj) to the name of the
subdirectory to build. Do the same for the packaging targets, otherwise
the build fails if $(srctree) is a relative directory:$ make O=build tar-pkg
make[1]: Entering directory `/home/mmarek/linux-2.6/build'
CHK include/config/kernel.release
../scripts/Makefile.build:44: ../../scripts/package/Makefile: No such file or directory
make[2]: *** No rule to make target `../../scripts/package/Makefile'. Stop.Fixes: 9da0763b ("kbuild: Use relative path when building in a subdir of the source tree")
Signed-off-by: Michal Marek
04 Jul, 2014
2 commits
-
Commit c2e28dc9 (kbuild: Print the name of the build directory) prints
the name of the build directory for O= builds, but we should not be
doing this in make -s mode, so that commands likemake -s O= kernelrelease
can be used by scripts. This matches the behavior of make itself, where
the -s option implies --no-print-directory.Signed-off-by: Michal Marek
-
Pull kbuild fix from Michal Marek:
"There is one more fix for the relative paths series from -rc1: Print
the path to the build directory at the start of the build, so that
editors and IDEs can match the relative paths to source files"* 'rc-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/mmarek/kbuild:
kbuild: Print the name of the build directory
03 Jul, 2014
1 commit
-
With commit 9da0763b (kbuild: Use relative path when building in a
subdir of the source tree), the compiler messages include relative
paths. These are however relative to the build directory, not the
directory where make was started. Print the "Entering directory ..."
message once, so that IDEs/editors can find the source files.Acked-by: David Howells
Signed-off-by: Michal Marek
30 Jun, 2014
1 commit
22 Jun, 2014
1 commit
16 Jun, 2014
1 commit
13 Jun, 2014
1 commit
-
Pull kbuild updates from Michal Marek:
"Kbuild changes for v3.16-rc1:- cross-compilation fix so that cc-option is testing the right
compiler
- Fix for make defconfig all
- Using relative paths to the object and source directory where
possible, plus fixes for the fallout of the change
- several cleanups in the Makefiles and scriptsThe powerpc fix is from today, because it was only discovered
recently. The rest has been in linux-next for some time"* 'kbuild' of git://git.kernel.org/pub/scm/linux/kernel/git/mmarek/kbuild:
powerpc: Avoid circular dependency with zImage.%
kbuild: create include/config directory in scripts/kconfig/Makefile
kbuild: do not create include/linux directory
Makefile: Fix unrecognized cross-compiler command line options
kbuild: do not add "selinux" to subdir- twice
um: Fix for relative objtree when generating x86 headers
kbuild: Use relative path when building in a subdir of the source tree
kbuild: Use relative path when building in the source tree
kbuild: Use relative path for $(objtree)
firmware: Use $(quote) in the Makefile
firmware: Simplify directory creation
kbuild: trivial - fix comment block indent
kbuild: trivial - remove trailing spaces
kbuild: support simultaneous "make %config" and "make all"
kbuild: move extra gcc checks to scripts/Makefile.extrawarn
10 Jun, 2014
3 commits
-
The directory include/config is used only for
silentoldconfig, localmodconfig, localyesconfig.Signed-off-by: Masahiro Yamada
Signed-off-by: Michal Marek -
There are no generated files under include/linux directory.
Signed-off-by: Masahiro Yamada
Signed-off-by: Michal Marek -
On architectures that setup CROSS_COMPILE in their arch/*/Makefile
(arc, blackfin, m68k, mips, parisc, score, sh, tile, unicore32, xtensa),
cc-option and cc-disable-warning may check against the wrong compiler,
causing errors likecc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
if the host gcc supports a compiler option, while the cross compiler
doesn't support that option.Move all logic using cc-option or cc-disable-warning below the inclusion
of the arch's Makefile to fix this.Introduced by
- commit e74fc973b6e531fef1fce8b101ffff05ecfb774c ("Turn off
-Wmaybe-uninitialized when building with -Os"),
- commit 61163efae02040f66a95c8ed17f4407951ba58fa ("kbuild: LLVMLinux:
Add Kbuild support for building kernel with Clang").As -Wno-maybe-uninitialized requires a quite recent gcc (gcc 4.6.3 on
Ubuntu 12.04 LTS doesn't support it), this only showed up recently (gcc
4.8.2 on Ubuntu 14.04 LTS does support it).Signed-off-by: Geert Uytterhoeven
Signed-off-by: Michal Marek
09 Jun, 2014
2 commits
-
Pull LLVM patches from Behan Webster:
"Next set of patches to support compiling the kernel with clang.
They've been soaking in linux-next since the last merge window.More still in the works for the next merge window..."
* tag 'llvmlinux-for-v3.16' of git://git.linuxfoundation.org/llvmlinux/kernel:
arm, unwind, LLVMLinux: Enable clang to be used for unwinding the stack
ARM: LLVMLinux: Change "extern inline" to "static inline" in glue-cache.h
all: LLVMLinux: Change DWARF flag to support gcc and clang
net: netfilter: LLVMLinux: vlais-netfilter
crypto: LLVMLinux: aligned-attribute.patch
08 Jun, 2014
1 commit
-
Both gcc (well, actually gnu as) and clang support the "-Wa,-gdwarf-2" option
(though clang does not support "-Wa,--gdwarf-2"). Since these flags are equivalent
in meaning, this patch uses the one which is better supported across compilers.Signed-off-by: Behan Webster
02 Jun, 2014
1 commit
26 May, 2014
1 commit
22 May, 2014
1 commit
15 May, 2014
3 commits
-
When doing make O=, use '..' to refer to the source tree. This
allows for more readable compiler messages, and, more importantly, it
sets the VPATH to '..', so filenames in WARN_ON() etc. will be shorter.Acked-by: Sam Ravnborg
Signed-off-by: Michal Marek -
When not using O=, $(srctree) refers to the same directory as
$(objtree), so we can set it to '.' as well. This makes the default
include path more compact and results in more readable messages from the
compiler. The only case where we need the absolute path is when creating
the 'source' symlink in /lib/modules.Acked-by: Sam Ravnborg
Signed-off-by: Michal Marek -
The main Makefile sets its working directory to the object tree and
never changes it again. Therefore, we can use '.' instead of the
absolute path. The only case where we need the absolute path is when
creating the 'build' symlink in /lib/modules.Acked-by: Sam Ravnborg
Signed-off-by: Michal Marek
10 May, 2014
1 commit
05 May, 2014
1 commit
30 Apr, 2014
3 commits
-
Signed-off-by: Masahiro Yamada
Signed-off-by: Michal Marek -
Signed-off-by: Masahiro Yamada
Signed-off-by: Michal Marek -
Kbuild is supposed to support mixed targets. (%config and build targets)
But "make all" did nothing if it was run with configuration targets.
For example,$ LANG=C make defconfig all
HOSTCC scripts/basic/fixdep
HOSTCC scripts/kconfig/conf.o
SHIPPED scripts/kconfig/zconf.tab.c
SHIPPED scripts/kconfig/zconf.lex.c
SHIPPED scripts/kconfig/zconf.hash.c
HOSTCC scripts/kconfig/zconf.tab.o
HOSTLD scripts/kconfig/conf
*** Default configuration is based on 'x86_64_defconfig'
#
# configuration written to .config
#
make: Nothing to be done for `all'.This commits allows "make %config all" and makes sure
mixed targets are built one by one in the given order.Signed-off-by: Masahiro Yamada
Cc: Michal Marek
CC: Sam Ravnborg
Signed-off-by: Michal Marek
28 Apr, 2014
1 commit
21 Apr, 2014
1 commit
17 Apr, 2014
1 commit
-
W=... provides extra gcc checks.
Having such code in scripts/Makefile.build results in the same flags
being added to KBUILD_CFLAGS multiple times becuase
scripts/Makefile.build is invoked every time Kbuild descends into
the subdirectories.Since the top Makefile is already too cluttered, this commit moves
all of extra gcc check stuff to a new file scripts/Makefile.extrawarn,
which is included from the top Makefile.Signed-off-by: Masahiro Yamada
CC: Sam Ravnborg
Signed-off-by: Michal Marek
14 Apr, 2014
1 commit
13 Apr, 2014
1 commit
-
Pull misc kbuild changes from Michal Marek:
"Here is the non-critical part of kbuild:
- One bogus coccinelle check removed, one check fixed not to suggest
the obsolete PTR_RET macro
- scripts/tags.sh does not index the generated *.mod.c files
- new objdiff tool to list differences between two versions of an
object file
- A fix for scripts/bootgraph.pl"* 'misc' of git://git.kernel.org/pub/scm/linux/kernel/git/mmarek/kbuild:
scripts/coccinelle: Use PTR_ERR_OR_ZERO
scripts/bootgraph.pl: Add graphic header
scripts: objdiff: detect object code changes between two commits
Coccicheck: Remove memcpy to struct assignment test
scripts/tags.sh: Ignore *.mod.c
10 Apr, 2014
1 commit
-
Add support to toplevel Makefile for compiling with clang, both for
HOSTCC and CC. Use cc-option to prevent gcc option from breaking clang, and
from clang options from breaking gcc.Clang 3.4 semantics are the same as gcc semantics for unsupported flags. For
unsupported warnings clang 3.4 returns true but shows a warning and gcc shows
a warning and returns false.Signed-off-by: Behan Webster
Signed-off-by: Jan-Simon Möller
Signed-off-by: Mark Charlebois
Cc: PaX Team