linux-kernel - Re: [PATCH 1/2] tracing: Move syscalls metadata handling from arch to core

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:	Sat, 19 Sep 2009 10:12:09 +0200
From:	Frederic Weisbecker <fweisbec@...il.com>
To:	Heiko Carstens <heiko.carstens@...ibm.com>
Cc:	Ingo Molnar <mingo@...e.hu>, LKML <linux-kernel@...r.kernel.org>,
	Steven Rostedt <rostedt@...dmis.org>,
	Li Zefan <lizf@...fujitsu.com>,
	Masami Hiramatsu <mhiramat@...hat.com>,
	Jason Baron <jbaron@...hat.com>,
	Lai Jiangshan <laijs@...fujitsu.com>,
	Martin Schwidefsky <schwidefsky@...ibm.com>,
	Paul Mundt <lethal@...ux-sh.org>
Subject: Re: [PATCH 1/2] tracing: Move syscalls metadata handling from arch
	to core

On Sat, Sep 19, 2009 at 09:48:16AM +0200, Heiko Carstens wrote:
> On Sat, Sep 19, 2009 at 07:39:16AM +0200, Frederic Weisbecker wrote:
> > Most of the syscalls metadata processing is done from arch.
> > But these operations are mostly generic accross archs. Especially now
> > that we have a common variable name that expresses the number of
> > syscalls supported by an arch: NR_syscalls, the only remaining bits
> > that need to reside in arch is the syscall nr to addr translation.
> 
> That won't work in its current form, since there is a small difference
> between x86 and s390:
> 
> > diff --git a/arch/s390/kernel/ftrace.c b/arch/s390/kernel/ftrace.c
> > index 57bdcb1..7c5752c 100644
> > --- a/arch/s390/kernel/ftrace.c
> > +++ b/arch/s390/kernel/ftrace.c
> > -static struct syscall_metadata *find_syscall_meta(unsigned long syscall)
> > -{
> > -	struct syscall_metadata *start;
> > -	struct syscall_metadata *stop;
> > -	char str[KSYM_SYMBOL_LEN];
> > -
> > -	start = (struct syscall_metadata *)__start_syscalls_metadata;
> > -	stop = (struct syscall_metadata *)__stop_syscalls_metadata;
> > -	kallsyms_lookup(syscall, NULL, NULL, NULL, str);
> > -
> > -	for ( ; start < stop; start++) {
> > -		if (start->name && !strcmp(start->name + 3, str + 3))
>                                            ^^^^^^^^^^^^^^^^^^^^^^^^



Oh, I thought it was to zap the "sys" prefix comparison.



> > diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
> > index 9dbb527..4adb867 100644
> > --- a/arch/x86/kernel/ftrace.c
> > +++ b/arch/x86/kernel/ftrace.c
> > -static struct syscall_metadata *find_syscall_meta(unsigned long *syscall)
> > -{
> > -	struct syscall_metadata *start;
> > -	struct syscall_metadata *stop;
> > -	char str[KSYM_SYMBOL_LEN];
> > -
> > -
> > -	start = (struct syscall_metadata *)__start_syscalls_metadata;
> > -	stop = (struct syscall_metadata *)__stop_syscalls_metadata;
> > -	kallsyms_lookup((unsigned long) syscall, NULL, NULL, NULL, str);
> > -
> > -	for ( ; start < stop; start++) {
> > -		if (start->name && !strcmp(start->name, str))
>                                            ^^^^^^^^^^^^^^^^
> > -			return start;
> > -	}
> > -	return NULL;
> 
> The reason for the "+ 3 " is that architectures with syscall wrappers have
> alias function names which also show up in kallsysms:
> 
> 000000000001c788 t show_cpuinfo
> 000000000001c9e0 T SyS_s390_personality
> 000000000001c9e0 T sys_s390_personality
> 000000000001ca48 T SyS_s390_newuname
> 000000000001ca48 T sys_s390_newuname
> 000000000001cac8 T SyS_ipc
> 000000000001cac8 T sys_ipc
> 000000000001cd00 T SyS_s390_old_mmap
> 000000000001cd00 T sys_s390_old_mmap
> 000000000001ce68 T SyS_mmap2
> 000000000001ce68 T sys_mmap2
> 000000000001cfc4 t FixPerRegisters
> 
> So kallsyms_lookup(...) would currently always return a string that starts
> with "SyS" instead of "sys". Since the metadata syscall names start with
> "sys" there is no match.
> If you could change the generic version so it also contains a "+ 3" it
> should work for all architectures.
> Might be worth a comment that I didn't add back then :)



Ah ok. So the fix is easy.
Thanks for the tip!


> Hmm... maybe it's even possible to throw out the "SyS" variants out of
> the kallsyms table and only keep the alias names?
> That would shrink the kernel image a bit.


Yeah, unless someone finds strong reasons to keep them.

Thanks!

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