lkml.org 
[lkml]   [2010]   [Jul]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH][GIT PULL][for 2.6.35] tracing: Add alignment to syscall metadata declarations
On Fri, Jul 09, 2010 at 03:56:42PM -0400, Steven Rostedt wrote:
>
> Ingo,
>
> Please pull the latest tip/perf/urgent tree, which can be found at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
> tip/perf/urgent
>
>
> Steven Rostedt (1):
> tracing: Add alignment to syscall metadata declarations
>
> ----
> include/linux/syscalls.h | 6 ++++--
> 1 files changed, 4 insertions(+), 2 deletions(-)
> ---------------------------
> commit 44a54f787c0abcf75a2ed49b8ec8b2b512468f73
> Author: Steven Rostedt <srostedt@redhat.com>
> Date: Fri Jul 9 15:41:44 2010 -0400
>
> tracing: Add alignment to syscall metadata declarations
>
> For some reason if we declare a static variable and then assign it
> later, and the assignment contains a __attribute__((__aligned__(#))),
> some versions of gcc will ignore it.
>
> This caused the syscall meta data to not be compact in its section
> and caused a kernel oops when the section was being read.
>
> The fix for these versions of gcc seems to be to add the aligned
> attribute to the declaration as well.
>
> This fixes the BZ regression:
>
> https://bugzilla.kernel.org/show_bug.cgi?id=16353
>
> Reported-by: Zeev Tarantov <zeev.tarantov@gmail.com>
> Tested-by: Zeev Tarantov <zeev.tarantov@gmail.com>
> Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
> LKML-Reference: <AANLkTinkKVmB0fpVeqUkMeqe3ZYeXJdI8xDuzJEOjYwh@mail.gmail.com>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
>
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index 7f614ce..13ebb54 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -124,7 +124,8 @@ extern struct trace_event_functions enter_syscall_print_funcs;
> extern struct trace_event_functions exit_syscall_print_funcs;
>
> #define SYSCALL_TRACE_ENTER_EVENT(sname) \
> - static struct syscall_metadata __syscall_meta_##sname; \
> + static struct syscall_metadata \
> + __attribute__((__aligned__(4))) __syscall_meta_##sname; \
> static struct ftrace_event_call \
> __attribute__((__aligned__(4))) event_enter_##sname; \
> static struct ftrace_event_call __used \
> @@ -138,7 +139,8 @@ extern struct trace_event_functions exit_syscall_print_funcs;
> }
>
> #define SYSCALL_TRACE_EXIT_EVENT(sname) \
> - static struct syscall_metadata __syscall_meta_##sname; \
> + static struct syscall_metadata \
> + __attribute__((__aligned__(4))) __syscall_meta_##sname; \
> static struct ftrace_event_call \
> __attribute__((__aligned__(4))) event_exit_##sname; \
> static struct ftrace_event_call __used \

This looks like a fix that just hide the real bug.
If I remember the original report correct the problem is
that the symbol:

__start_syscalls_metadata

Does not point to a valid syscall entry.

The symbol is assigned in vmlinux.lds.h like this:
#define TRACE_SYSCALLS() VMLINUX_SYMBOL(__start_syscalls_metadata) = .; \
*(__syscalls_metadata) \
VMLINUX_SYMBOL(__stop_syscalls_metadata) = .;

Now consider what is happening if we have the following scanario:

. equals 0x1004 so __start_syscalls_metadata is set to 0x1004
But __syscall_metadata require 8 byte alignment so it starts at 0x1008.

Then __start_syscalls_metadata fails to point at the first entry and
this code will fail:

start = (struct syscall_metadata *)__start_syscalls_metadata;
for ( ; start < stop; start++) {
if (start->name && !strcmp(start->name + 3, str + 3))
return start;


iThe right fix seems to me that we guarantee that __start_syscalls_metadata
is assinged a proper address.

Something like this:
(whitespace damaged)
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 48c5299..64430d3 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -133,7 +133,8 @@
#endif

#ifdef CONFIG_FTRACE_SYSCALLS
-#define TRACE_SYSCALLS() VMLINUX_SYMBOL(__start_syscalls_metadata) = .; \
+#define TRACE_SYSCALLS() . = ALIGN(8); \
+ VMLINUX_SYMBOL(__start_syscalls_metadata) = .; \
*(__syscalls_metadata) \
VMLINUX_SYMBOL(__stop_syscalls_metadata) = .;
#else

But at the same time we should audit the other trace related symbols
in vmlinux.lds.h to see if they may suffer from the same potential bug.

__start_ftrace_events looks like it may have the same potential bug..

The reason why your patch works is that you force a smaller alignment
and this happens by pure luck to be the alignmnet of "." from the
previous section.

Sam


\
 
 \ /
  Last update: 2010-07-09 22:35    [from the cache]
©2003-2011 Jasper Spaans