linux-kernel - Re: [PATCH 4/6] ftrace syscalls: Allow arch specific syscall symbol matching

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: <AANLkTik7O=xVP2WuD+AV-oQqg9heT4fVbEa65W+BRq11@mail.gmail.com>
Date:	Mon, 6 Dec 2010 23:56:49 -0500
From:	Mike Frysinger <vapier@...too.org>
To:	Ian Munsie <imunsie@....ibm.com>
Cc:	Avantika Mathur <mathur@...ux.vnet.ibm.com>,
	Jason Baron <jbaron@...hat.com>,
	Randy Dunlap <rdunlap@...otime.net>,
	Steven Rostedt <rostedt@...dmis.org>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Ingo Molnar <mingo@...hat.com>,
	Heiko Carstens <heiko.carstens@...ibm.com>,
	linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/6] ftrace syscalls: Allow arch specific syscall symbol matching

On Mon, Dec 6, 2010 at 23:29, Ian Munsie wrote:
> From: Ian Munsie <imunsie@....ibm.com>
>
> Some architectures have unusual symbol names and the generic code to
> match the symbol name with the function name for the syscall metadata
> will fail. For example, symbols on PPC64 start with a period and the
> generic code will fail to match them.
>
> This patch splits out the match logic into a standalone weak function
> that can be overridden on archs with unusual symbol names.
>
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -527,6 +527,7 @@ extern enum ftrace_dump_mode ftrace_dump_on_oops;
>  #ifdef CONFIG_FTRACE_SYSCALLS
>
>  unsigned long arch_syscall_addr(int nr);
> +bool arch_syscall_match_sym_name(const char *sym, const char *name);
>
>  #endif /* CONFIG_FTRACE_SYSCALLS */
>
> --- a/kernel/trace/trace_syscalls.c
> +++ b/kernel/trace/trace_syscalls.c
> @@ -81,13 +81,7 @@ static struct syscall_metadata *find_syscall_meta(unsigned long syscall)
>        kallsyms_lookup(syscall, NULL, NULL, NULL, str);
>
>        for ( ; start < stop; start++) {
> -               /*
> -                * Only compare after the "sys" prefix. Archs that use
> -                * syscall wrappers may have syscalls symbols aliases prefixed
> -                * with "SyS" instead of "sys", leading to an unwanted
> -                * mismatch.
> -                */
> -               if (start->name && !strcmp(start->name + 3, str + 3))
> +               if (start->name && arch_syscall_match_sym_name(str, start->name))
>                        return start;
>        }
>        return NULL;
> @@ -452,6 +446,17 @@ unsigned long __init __weak arch_syscall_addr(int nr)
>        return (unsigned long)sys_call_table[nr];
>  }
>
> +bool __weak arch_syscall_match_sym_name(const char *sym, const char *name)
> +{
> +       /*
> +        * Only compare after the "sys" prefix. Archs that use
> +        * syscall wrappers may have syscalls symbols aliases prefixed
> +        * with "SyS" instead of "sys", leading to an unwanted
> +        * mismatch.
> +        */
> +       return (!strcmp(sym + 3, name + 3));
> +}

useless set of parenthesis, and this overhead sucks.  weak +
additional function call just for a strcmp for most people ?  why not
make it into a define in the header:
#ifndef arch_syscall_match_sym_name
#define arch_syscall_match_sym_name(sym, name) !strcmp(sym + 3, name + 3)
#endif
-mike
--
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/