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]
Message-ID: <20110127181907.GA5226@jolsa.brq.redhat.com>
Date:	Thu, 27 Jan 2011 19:19:07 +0100
From:	Jiri Olsa <jolsa@...hat.com>
To:	Steven Rostedt <rostedt@...dmis.org>
Cc:	linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...e.hu>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Frederic Weisbecker <fweisbec@...il.com>,
	David Miller <davem@...emloft.net>,
	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Subject: Re: [PATCH 3/3] tracepoints: Use __u64_aligned/U64_ALIGN()

On Tue, Jan 25, 2011 at 11:05:02PM -0500, Steven Rostedt wrote:
> From: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
> 
> commit 7e066fb870fcd1025ec3ba7bbde5d541094f4ce1 added the aligned(32) type and
> variable attribute to the tracepoint structures to deal with gcc happily
> aligning statically defined structures on 32-byte multiples.
> 
> Working on issues within Ftrace, we came up with __64_aligned, which deals with
> this issue more elegantly by forcing an 8-byte alignment to both the type
> declaration and variable definition.
> 
> This therefore saves space, bringing down the size of struct tracepoint from 64
> bytes to 38 on 64-bit architectures.
> 
> Updating:
> - The type attribute (for iteration over the struct tracepoint array)
> - Added the variable attribute to the extern definition (needed to force gcc to
>   consider this alignment for the following definition)
> - The definition variable attribute (to force gcc to this specific alignment for
>   the static definitions)
> - The linker script (8-byte alignment can now replace the previous 32-byte
>   alignment for the custom tracepoint section)
> 
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
> LKML-Reference: <20110121203643.046218322@...icios.com>
> Acked-by: David Miller <davem@...emloft.net>
> CC: Frederic Weisbecker <fweisbec@...il.com>
> CC: Ingo Molnar <mingo@...e.hu>
> Signed-off-by: Steven Rostedt <rostedt@...dmis.org>
> ---
>  include/asm-generic/vmlinux.lds.h |    2 +-
>  include/linux/tracepoint.h        |   12 ++++--------
>  2 files changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index e4af65c..95abb5f 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -176,7 +176,7 @@
>  	CPU_KEEP(exit.data)						\
>  	MEM_KEEP(init.data)						\
>  	MEM_KEEP(exit.data)						\
> -	. = ALIGN(32);							\
> +	U64_ALIGN();							\
>  	VMLINUX_SYMBOL(__start___tracepoints) = .;			\
>  	*(__tracepoints)						\
>  	VMLINUX_SYMBOL(__stop___tracepoints) = .;			\
> diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
> index c681461..4bc50ea 100644
> --- a/include/linux/tracepoint.h
> +++ b/include/linux/tracepoint.h
> @@ -33,12 +33,7 @@ struct tracepoint {
>  	void (*regfunc)(void);
>  	void (*unregfunc)(void);
>  	struct tracepoint_func __rcu *funcs;
> -} __attribute__((aligned(32)));		/*
> -					 * Aligned on 32 bytes because it is
> -					 * globally visible and gcc happily
> -					 * align these on the structure size.
> -					 * Keep in sync with vmlinux.lds.h.
> -					 */
> +} __u64_aligned;
>  
>  /*
>   * Connect a probe to a tracepoint.
> @@ -146,7 +141,7 @@ static inline void tracepoint_update_probe_range(struct tracepoint *begin,
>   * structure. Force alignment to the same alignment as the section start.
>   */
>  #define __DECLARE_TRACE(name, proto, args, cond, data_proto, data_args)	\
> -	extern struct tracepoint __tracepoint_##name;			\
> +	extern struct tracepoint __u64_aligned __tracepoint_##name;	\
>  	static inline void trace_##name(proto)				\
>  	{								\
>  		JUMP_LABEL(&__tracepoint_##name.state, do_trace);	\
> @@ -178,7 +173,8 @@ do_trace:								\
>  	static const char __tpstrtab_##name[]				\
>  	__attribute__((section("__tracepoints_strings"))) = #name;	\
>  	struct tracepoint __tracepoint_##name				\
> -	__attribute__((section("__tracepoints"), aligned(32))) =	\
> +	__u64_aligned							\
> +	__attribute__((section("__tracepoints"))) =			\
>  		{ __tpstrtab_##name, 0, reg, unreg, NULL }
>  
>  #define DEFINE_TRACE(name)						\
> -- 
> 1.7.2.3

hi,

this patch breaks tracepoint section iterating for me
when trying to enable any event I get:

[   69.119483] BUG: unable to handle kernel NULL pointer dereference at           (null)
[   69.119868] IP: [<ffffffff8121c2c2>] strlen+0x2/0x30
[   69.120104] PGD 77b84067 PUD 75dd6067 PMD 0 
[   69.120104] Oops: 0000 [#1] SMP 
[   69.120104] last sysfs file: /sys/devices/virtual/vc/vcs5/uevent
[   69.120104] CPU 1 
[   69.120104] Modules linked in:
[   69.120104] 
[   69.120104] Pid: 1119, comm: bash Not tainted 2.6.38-rc2-tip+ #325 To be filled by O.E.M./Montevina platform
[   69.120104] RIP: 0010:[<ffffffff8121c2c2>]  [<ffffffff8121c2c2>] strlen+0x2/0x30
[   69.120104] RSP: 0018:ffff8800754fbd60  EFLAGS: 00010246
[   69.120104] RAX: 0000000000000000 RBX: ffffffff8180e530 RCX: 0000000073ed290b
[   69.120104] RDX: 0000000002687d93 RSI: ffffffff8173387c RDI: 0000000000000000
[   69.120104] RBP: ffff8800754fbd88 R08: 0000000000000000 R09: 0000000000000000
[   69.120104] R10: ffff8800779dbad0 R11: 0000000000000000 R12: 0000000000000000
[   69.120104] R13: 0000000000000000 R14: ffffffff8180fa48 R15: 0000000000000000
[   69.120104] FS:  00007fd6c90ae720(0000) GS:ffff88007b480000(0000) knlGS:0000000000000000
[   69.120104] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   69.120104] CR2: 0000000000000000 CR3: 0000000075bec000 CR4: 00000000000406e0
[   69.120104] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[   69.120104] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[   69.120104] Process bash (pid: 1119, threadinfo ffff8800754fa000, task ffff88007bdc2640)
[   69.120104] Stack:
[   69.120104]  ffffffff8109b9fa ffff8800779dbb00 ffffffff8180e530 0000000000000000
[   69.120104]  0000000000000001 ffff8800754fbdc8 ffffffff8109c4b6 ffffffff81a1d3e8
[   69.120104]  0000000000000000 0000000000000000 0000000000000000 ffff880076ed3c00
[   69.120104] Call Trace:
[   69.120104]  [<ffffffff8109b9fa>] ? get_tracepoint+0x1a/0x1a0
[   69.120104]  [<ffffffff8109c4b6>] tracepoint_update_probe_range+0xe6/0x1d0
[   69.120104]  [<ffffffff8109c5fc>] tracepoint_update_probes+0x1c/0x30
[   69.120104]  [<ffffffff8109c855>] tracepoint_probe_register+0x85/0xb0
[   69.120104]  [<ffffffff810ad4dd>] tracing_start_sched_switch+0x4d/0xf0
[   69.120104]  [<ffffffff810ad5f9>] tracing_start_cmdline_record+0x9/0x10
[   69.120104]  [<ffffffff810b5955>] ftrace_event_enable_disable+0xc5/0xf0
[   69.120104]  [<ffffffff810b6648>] event_enable_write+0xf8/0x130
[   69.120104]  [<ffffffff8110e564>] ? rw_verify_area+0xf4/0x1b0
[   69.120104]  [<ffffffff8110f0d9>] vfs_write+0x119/0x1c0
[   69.120104]  [<ffffffff8110f281>] sys_write+0x51/0x90
[   69.120104]  [<ffffffff8100316b>] system_call_fastpath+0x16/0x1b
[   69.120104] Code: 11 eb 1b 66 0f 1f 44 00 00 48 83 ea 01 48 39 c2 72 0c 0f b6 0a f6 81 a0 c4 55 81 20 75 eb c6 42 01 00 c9 c3 0f 1f 44 00 00 31 c0 <80> 3f 00 55 48 89 fa 48 89 e5 74 11 66 90 48 83 c2 01 80 3a 00 
[   69.120104] RIP  [<ffffffff8121c2c2>] strlen+0x2/0x30
[   69.120104]  RSP <ffff8800754fbd60>
[   69.120104] CR2: 0000000000000000



tracepoint_update_probe_range will oops due to the tracepoint name
being null

I found the elements in "__tracepoints" sections might have different
size after above patch is applied:

with:

nm vmlinux | grep __tracepoint_ | sort

I got:

...
ffffffff8180e4b8 D __tracepoint_clock_disable
ffffffff8180e4e0 D __tracepoint_clock_set_rate
ffffffff8180e508 D __tracepoint_power_domain_target
ffffffff8180e540 D __tracepoint_mm_vmscan_kswapd_sleep
ffffffff8180e568 D __tracepoint_mm_vmscan_kswapd_wake
...

addr(__tracepoint_mm_vmscan_kswapd_sleep) - addr(__tracepoint_power_domain_target) = 0x38

while size of "struct tracepoints" seems to be 0x28

and the listing has several places like that


At least the code "tracepoint_update_probe_range" relies on that,
while iterating the section.

please let me know if you need more info,
not sure what's going on.. :)

thanks,
jirka


[root@...p-26-214 ~]# uname -a
Linux dhcp-26-214.brq.redhat.com 2.6.38-rc2-tip+ #325 SMP Thu Jan 27 19:11:56 CET 2011 x86_64 x86_64 x86_64 GNU/Linux

[jolsa@...va1 linux-2.6-tip]$ gcc --version
gcc (GCC) 4.4.4 20100726 (Red Hat 4.4.4-13)
Copyright (C) 2010 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
PURPOSE.

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