| 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
| ||
|
Date: Mon, 17 Jan 2011 22:00:39 -0800 (PST)
From: David Miller <davem@...emloft.net>
To: rostedt@...dmis.org
Cc: richm@...elvet.org.uk, 609371@...s.debian.org, ben@...adent.org.uk,
sparclinux@...r.kernel.org, linux-kernel@...r.kernel.org,
fweisbec@...il.com, mingo@...hat.com,
mathieu.desnoyers@...icios.com
Subject: Re: Bug#609371: linux-image-2.6.37-trunk-sparc64: module scsi_mod:
Unknown relocation: 36
From: David Miller <davem@...emloft.net>
Date: Mon, 17 Jan 2011 21:34:48 -0800 (PST)
> Where are these "holes" coming from? Reading the commit message for
> the change that introduced this problem
> (86c38a31aa7f2dd6e74a262710bf8ebf7455acc5), it seems like the issue is
> coming from the compiler, and that fact that beforehand some
> structures were marked with 4-byte alignment. The existing 4-byte
> alignment cases sound like the real bug to me, where is that happening
> and why?
Ok, now I'm getting a bit irritated. I went and looked into the
history of this "aligned(4)" tag on ftrace_event_call.
The reason for it is completely undocumented, and in fact it gets
added in a commit whose commit message doesn't mention this part
of the commit's change at all.
That really sucks.
The commit message is:
commit 1473e4417c79f12d91ef91a469699bfa911f510f
Author: Steven Rostedt <srostedt@...hat.com>
Date: Tue Feb 24 14:15:08 2009 -0500
tracing: make event directory structure
This patch adds the directory /debug/tracing/events/ that will contain
all the registered trace points.
# ls /debug/tracing/events/
sched_kthread_stop sched_process_fork sched_switch
sched_kthread_stop_ret sched_process_free sched_wait_task
sched_migrate_task sched_process_wait sched_wakeup
sched_process_exit sched_signal_send sched_wakeup_new
# ls /debug/tracing/events/sched_switch/
enable
# cat /debug/tracing/events/sched_switch/enable
1
# cat /debug/tracing/set_event
sched_switch
Signed-off-by: Steven Rostedt <srostedt@...hat.com>
And in this commit we have:
@@ -39,6 +41,7 @@ static void ftrace_unreg_event_##call(void) \
} \
\
static struct ftrace_event_call __used \
+__attribute__((__aligned__(4))) \
__attribute__((section("_ftrace_events"))) event_##call = { \
.name = #call, \
.regfunc = ftrace_reg_event_##call, \
Excuse me?
This is a serious infraction of responsible development, and it makes
bugs hard if not impossible to analyze. I see it was postedo on LKML,
and I'm surprised nobody asked "Why are you adding that alignment, and
whatever the reason why isn't it mentioned in the commit message or in
a comment?"
Anyways, this commit is where this alignment originates.
It probably is spurious and unnecessary and we can remove it
completely from _ALL_ of the cases. Did anyone do any research
whatsoever into why the alignment tag was put there in the first place
when the GCC 4.5 bug showed up?
Steven what is the deal here?
Can't we just do the following patch? Also, I notice similar
alignment tag being used for syscall_metadata, what's the story there?
--------------------
ftrace: Remove unnecessary alignment tag from ftrace_event_call.
It's completely unnecessary and causes problems on platforms
where this tag down-aligns the structure's alignment.
Signed-off-by: David S. Miller <davem@...emloft.net>
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 18cd068..05295f2 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -128,7 +128,6 @@ extern struct trace_event_functions exit_syscall_print_funcs;
static struct syscall_metadata \
__attribute__((__aligned__(4))) __syscall_meta_##sname; \
static struct ftrace_event_call __used \
- __attribute__((__aligned__(4))) \
__attribute__((section("_ftrace_events"))) \
event_enter_##sname = { \
.name = "sys_enter"#sname, \
@@ -142,7 +141,6 @@ extern struct trace_event_functions exit_syscall_print_funcs;
static struct syscall_metadata \
__attribute__((__aligned__(4))) __syscall_meta_##sname; \
static struct ftrace_event_call __used \
- __attribute__((__aligned__(4))) \
__attribute__((section("_ftrace_events"))) \
event_exit_##sname = { \
.name = "sys_exit"#sname, \
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index e16610c..bd86d1b 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -68,8 +68,7 @@
#undef DEFINE_EVENT
#define DEFINE_EVENT(template, name, proto, args) \
- static struct ftrace_event_call __used \
- __attribute__((__aligned__(4))) event_##name
+ static struct ftrace_event_call __used event_##name
#undef DEFINE_EVENT_PRINT
#define DEFINE_EVENT_PRINT(template, name, proto, args, print) \
@@ -447,7 +446,6 @@ static inline notrace int ftrace_get_offsets_##call( \
* };
*
* static struct ftrace_event_call __used
- * __attribute__((__aligned__(4)))
* __attribute__((section("_ftrace_events"))) event_<call> = {
* .name = "<call>",
* .class = event_class_<template>,
@@ -580,7 +578,6 @@ static struct ftrace_event_class __used event_class_##call = { \
#define DEFINE_EVENT(template, call, proto, args) \
\
static struct ftrace_event_call __used \
-__attribute__((__aligned__(4))) \
__attribute__((section("_ftrace_events"))) event_##call = { \
.name = #call, \
.class = &event_class_##template, \
@@ -594,7 +591,6 @@ __attribute__((section("_ftrace_events"))) event_##call = { \
static const char print_fmt_##call[] = print; \
\
static struct ftrace_event_call __used \
-__attribute__((__aligned__(4))) \
__attribute__((section("_ftrace_events"))) event_##call = { \
.name = #call, \
.class = &event_class_##template, \
--
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