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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Tue, 6 Oct 2009 16:32:26 -0400
From:	Jason Baron <jbaron@...hat.com>
To:	Steven Rostedt <rostedt@...dmis.org>
Cc:	Justin Mattock <justinmattock@...il.com>,
	Ingo Molnar <mingo@...e.hu>,
	Peter Zijlstra <peterz@...radead.org>,
	Li Zefan <lizf@...fujitsu.com>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: system gets stuck in a lock during boot

On Mon, Oct 05, 2009 at 09:24:09PM -0400, Steven Rostedt wrote:
> On Fri, 2009-10-02 at 17:12 -0400, Jason Baron wrote:
> 
> > hi Justin,
> > 
> > I've been playing around with gcc '4.5' as well and hit a panic that
> > looks very similar to what you've seen with stock 2.6.31 - I haven't
> > seen it anywhere else. Anyways, it seems to be some sort of alignment
> > issue with the 'struct ftrace_event_call'. I'm not sure yet if this is a
> > compiler or kernel issue. But the following kernel patch fixes the issue
> > for me. It would be interesting to verify if the patch also resolves the
> > issue for you.
> > 
> > thanks,
> > 
> > -Jason
> > 
> > 
> > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> > index 6ad76bf..0029af4 100644
> > --- a/include/asm-generic/vmlinux.lds.h
> > +++ b/include/asm-generic/vmlinux.lds.h
> > @@ -164,6 +164,7 @@
> >  	LIKELY_PROFILE()		       				\
> >  	BRANCH_PROFILE()						\
> >  	TRACE_PRINTKS()							\
> > +	. = ALIGN(32);							\
> >  	FTRACE_EVENTS()							\
> >  	TRACE_SYSCALLS()
> >  
> > diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
> > index a81170d..43f9f1e 100644
> > --- a/include/linux/ftrace_event.h
> > +++ b/include/linux/ftrace_event.h
> > @@ -124,7 +124,7 @@ struct ftrace_event_call {
> >  	atomic_t		profile_count;
> >  	int			(*profile_enable)(struct ftrace_event_call *);
> >  	void			(*profile_disable)(struct ftrace_event_call *);
> > -};
> > +} __attribute__((aligned(32)));
> >  
> >  #define MAX_FILTER_PRED		32
> >  #define MAX_FILTER_STR_VAL	128
> > diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
> > index f64fbaa..4697fb6 100644
> > --- a/include/trace/ftrace.h
> > +++ b/include/trace/ftrace.h
> > @@ -600,7 +600,7 @@ static int ftrace_raw_init_event_##call(void)				\
> >  }									\
> >  									\
> >  static struct ftrace_event_call __used					\
> > -__attribute__((__aligned__(4)))						\
> > +__attribute__((__aligned__(32)))					\
> >  __attribute__((section("_ftrace_events"))) event_##call = {		\
> >  	.name			= #call,				\
> >  	.system			= __stringify(TRACE_SYSTEM),		\
> 
> Are all alignments needed? Or just adding one might help. Or removing
> the one directly above?
> 
> -- Steve
> 

So the problem I'm seeing is an oops on boot caused by the call->system pointer
deference in event_create_dir(). The 'call' variable is of type 'struct
ftrace_event_call'. 

What's going on is that the 'struct ftrace_event_call' is of size 168 bytes
(sizeof(struct ftrace_event_call)) = 168 = 0xA8. However, in memory the
structures are 16-byte aligned. Thus, the stride for walking through the
pointers needs to be 176 (0xB0), but instead its 168 causing the oops.

I've only seen this issue while using gcc (GCC) 4.5.0 20090916, on a
vanilla 2.6.31 kernel.

That said, I'm not sure the compiler is doing the wrong thing here. The
'struct ftrace_event_call' contains an embedded 'struct list_head' which
is 16 bytes. According to the gcc docs, the aligned attribute, 'specifies a
minimum alignment for the variable or structure field, measured in bytes'.
Thus, at least according to the docs, gcc can increase the alignment of the
'struct ftrace_event_call', from its original specification of 4, to 16. Even
in the case where we are working corectly the structures are 8-byte aligned.

Thus, I would reccommend the patch below as a preventive measure. Its
the minimal patch I've found to resolve this issue. In general, if we
are going to walk data structures embedded in a special elf section, I
think the general rules needs to be to set the alignment to the power of
two which is greater than or equal to the largest item in the structure.

thanks,

-Jason

Signed-off-by: Jason Baron <jbaron@...hat.com>


diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index a81170d..7182f03 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -124,7 +124,10 @@ struct ftrace_event_call {
 	atomic_t		profile_count;
 	int			(*profile_enable)(struct ftrace_event_call *);
 	void			(*profile_disable)(struct ftrace_event_call *);
-};
+} __attribute__((aligned(16)));
+
+/* Align to the largest field in the data structure:
+ * sizeof(struct list_head) = 16 */
 
 #define MAX_FILTER_PRED		32
 #define MAX_FILTER_STR_VAL	128
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index f64fbaa..e344e81 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -600,7 +600,6 @@ static int ftrace_raw_init_event_##call(void)				\
 }									\
 									\
 static struct ftrace_event_call __used					\
-__attribute__((__aligned__(4)))						\
 __attribute__((section("_ftrace_events"))) event_##call = {		\
 	.name			= #call,				\
 	.system			= __stringify(TRACE_SYSTEM),		\



--
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

Powered by Openwall GNU/*/Linux Powered by OpenVZ