kallsyms: Nuke the SyS_* aliases

RFC PATCH In review Steven Rostedt 3 participants 9 messages May 6, 2015 – Jun 15, 2017

Subsystems (2)

Kbuild ABI/API

lore

Share & tools

lore

Steven Rostedt

author

2015-05-06 18:24 UTC #0 lore raw

cover kallsyms: Nuke the SyS_* aliases

This has been on my todo list for some time (years), and it has annoyed me enough that I finally decided to look at how to handle it.

The issue is the SyS_foo() system call wrapper aliases that represent each sys_foo() system call. Their purpose is to force a sign extend of each argument for archs that require it. As I believe that the SyS_foo() protocol only has a bunch of longs as its arguments that it can not be a replacement for sys_foo(), otherwise the compiler will complain about the mismatch arguments. Instead, the real system call is actually a static inline of SYS_foo() and SyS_foo() calls it by typecasting each long argument to the proper type that the system call expects. Then sys_foo() is just aliased to SyS_foo() such that calls to sys_foo() will really call SyS_foo(), which will do the proper sign extension.

The above pretty much sums up why system calls have those strange SYSCALL_DEFINEx() macros.

The issue is that SyS_foo() pops up all over the place in kallsyms and for filtering of functions in the function and function graph tracers. And with live kernel patching, it may become even more invasive.

Al Viro mentioned nuking this in the past too:

http://lkml.kernel.org/r/[email protected]

This series is to nuke it from kallsyms, and that kallsyms_lookup() does not see it either. That will also take it out of the function tracing filtering mechanism.

This does not touch the compat_SyS_foo() calls though.

The first patch changes SyS_foo() to static such that nothing outside the file can reference it.

The second patch changes scripts/kallsyms.c to ignore the SyS_foo() calls if sys_foo() comes before or after it.

git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
kallsyms

Head SHA1: b2faf7432b822aa9b6e5c01a09f28a26102fdf02

Steven Rostedt (Red Hat) (2):
syscalls.h: Make SyS_foo() declaration static
kallsyms: Do not display SyS_foo() syscall aliases in kallsyms

----

 include/linux/syscalls.h |  3 +--
 scripts/kallsyms.c       | 43 ++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 39 insertions(+), 7 deletions(-)

Steven Rostedt

author

2015-05-06 18:24 UTC #1 lore raw

RFC PATCH 1/2 syscalls.h: Make SyS_foo() declaration static

From: "Steven Rostedt (Red Hat)" <[email protected]>

Nothing should call the syscall wrapper alias (SyS_foo) directly. It is only used to force the system call to sign extend its arguments. See commit 1a94bc34768e "System call wrapper infrastructure".

By declaring it static, we can also remove the protoype that was added by commit f9597f24c089 "syscalls.h: add forward declarations for inplace syscall wrappers", that fixed the warnings that were generated because the call was not static.

Cc Christian Borntraeger <[email protected]>

Cc Martin Schwidefsky <[email protected]>

Cc Heiko Carstens <[email protected]>

Cc Sergei Trofimovich <[email protected]>

Cc Al Viro <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 include/linux/syscalls.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

include/linux/syscalls.h +1 -2

index 76d1e38aabe1..f8e5b696670c 100644
@@ -195,8 +195,7 @@ extern struct trace_event_functions exit_syscall_print_funcs;
 	asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))	\
 		__attribute__((alias(__stringify(SyS##name))));		\
 	static inline long SYSC##name(__MAP(x,__SC_DECL,__VA_ARGS__));	\
-	asmlinkage long SyS##name(__MAP(x,__SC_LONG,__VA_ARGS__));	\
-	asmlinkage long SyS##name(__MAP(x,__SC_LONG,__VA_ARGS__))	\
+	asmlinkage static long SyS##name(__MAP(x,__SC_LONG,__VA_ARGS__))	\
 	{								\
 		long ret = SYSC##name(__MAP(x,__SC_CAST,__VA_ARGS__));	\
 		__MAP(x,__SC_TEST,__VA_ARGS__);				\

Signature · 1 line

2.1.4

Steven Rostedt

author

2015-05-06 18:24 UTC #2 lore raw

RFC PATCH 2/2 kallsyms: Do not display SyS_foo() syscall aliases in kallsyms

In reply to Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

The SyS_foo() alias wrapper was added to make sure that system call arguments were signed extended. The call itself is to never be used by anything, only the sys_foo() version is. But this symbol is stored in /proc/kallsyms, and is returned sometimes as the name of system call functions when a ksym lookup is made, it confuses the function tracer interface (see available_filter_functions in the tracefs directory).

Al Viro even suggested that this should be removed from kallsyms as well:

Link http://lkml.kernel.org/r/[email protected] open here

Modify the compile time kallsyms.c to check if the function name begins with SyS_ and is before or after the same name that starts with sys_ and if so, do not record it. This saves some space and more importantly removes the confusing variations of the system call name.

wc kallsyms.*
90151 284644 3819255 kallsyms.orig
89826 283669 3808628 kallsyms.patched

size vmlinux*
text data bss dec hex filename
9990933 2368592 1249280 13608805 cfa765 vmlinux.orig
9986837 2368592 1249280 13604709 cf9765 vmlinux.patched

This patch only removes SyS_*, it does not do anything with compat_SyS_*.

Cc Al Viro <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 scripts/kallsyms.c | 43 ++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 38 insertions(+), 5 deletions(-)

scripts/kallsyms.c +38 -5

index 8fa81e84e295..a64d89c6641c 100644
@@ -193,8 +193,22 @@ static int symbol_in_range(struct sym_entry *s, struct addr_range *ranges,
 	return 0;
 }
 
-static int symbol_valid(struct sym_entry *s)
+static const char *skip_prefix(const char *sym)
 {
+	if (symbol_prefix_char && *sym == symbol_prefix_char)
+		return sym + 1;
+	return sym;
+}
+
+static int match_sys(const char *sym, const char *sys)
+{
+	return !strncmp(sys, "sys_", 4) && !strcmp(sym + 4, sys + 4);
+}
+
+static int symbol_valid(int idx)
+{
+	struct sym_entry *s = &table[idx];
+
 	/* Symbols which vary between passes.  Passes 1 and 2 must have
 	 * identical symbol lists.  The kallsyms_* symbols below are only added
 	 * after pass 1, they would be included in pass 2 when --all-symbols is

@@ -224,9 +238,7 @@ static int symbol_valid(struct sym_entry *s)
 	if (s->addr < kernel_start_addr)
 		return 0;
 
-	/* skip prefix char */
-	if (symbol_prefix_char && *sym_name == symbol_prefix_char)
-		sym_name++;
+	sym_name = skip_prefix(sym_name);
 
 
 	/* if --all-symbols is not specified, then symbols outside the text

@@ -255,6 +267,27 @@ static int symbol_valid(struct sym_entry *s)
 		if (strcmp(sym_name, special_symbols[i]) == 0)
 			return 0;
 
+	/* Ignore SyS_* alias system calls */
+	if (!strncmp(sym_name, "SyS_", 4)) {
+		char *sym_name_before = "";
+		char *sym_name_after = "";
+
+		if (i > 0) {
+			sym_name_before = (char *)table[idx-1].sym + 1;
+			sym_name_before = skip_prefix(sym_name_before);
+		}
+
+		if (i < table_cnt - 1) {
+			sym_name_after = (char *)table[idx+1].sym + 1;
+			sym_name_after = skip_prefix(sym_name_after);
+		}
+
+		/* If SyS_foo matches a sys_foo, skip it */
+		if (match_sys(sym_name, sym_name_before) ||
+		    match_sys(sym_name, sym_name_after))
+			return 0;
+	}
+
 	for (i = 0; special_suffixes[i]; i++) {
 		int l = strlen(sym_name) - strlen(special_suffixes[i]);

@@ -447,7 +480,7 @@ static void build_initial_tok_table(void)
 
 	pos = 0;
 	for (i = 0; i < table_cnt; i++) {
-		if ( symbol_valid(&table[i]) ) {
+		if ( symbol_valid(i) ) {
 			if (pos != i)
 				table[pos] = table[i];
 			learn_symbol(table[pos].sym, table[pos].len);

Signature · 1 line

2.1.4

Borislav Petkov

2015-05-22 10:55 UTC #3 lore raw

Re: [RFC][PATCH 2/2] kallsyms: Do not display SyS_foo() syscall aliases in kallsyms

Quoted 44 lines · Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>
The SyS_foo() alias wrapper was added to make sure that system call arguments were signed extended. The call itself is to never be used by anything, only the sys_foo() version is. But this symbol is stored in /proc/kallsyms, and is returned sometimes as the name of system call functions when a ksym lookup is made, it confuses the function tracer interface (see available_filter_functions in the tracefs directory).
Al Viro even suggested that this should be removed from kallsyms as well:
Link http://lkml.kernel.org/r/[email protected] open here
Modify the compile time kallsyms.c to check if the function name begins with SyS_ and is before or after the same name that starts with sys_ and if so, do not record it. This saves some space and more importantly removes the confusing variations of the system call name.
wc kallsyms.*
90151 284644 3819255 kallsyms.orig
89826 283669 3808628 kallsyms.patched
size vmlinux*
text data bss dec hex filename
9990933 2368592 1249280 13608805 cfa765 vmlinux.orig
9986837 2368592 1249280 13604709 cf9765 vmlinux.patched
This patch only removes SyS_*, it does not do anything with compat_SyS_*.
Cc Al Viro <[email protected]>
Signed-off-by Steven Rostedt <[email protected]>
---
 scripts/kallsyms.c | 43 ++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 38 insertions(+), 5 deletions(-)
scripts/kallsyms.c +0 -0
index 8fa81e84e295..a64d89c6641c 100644

scripts/kallsyms.c static int symbol_in_range(struct sym_entry *s, struct addr_range *ranges, · line 201

Quoted 10 lines

scripts/kallsyms.c +5 -1

@@ -193,8 +193,22 @@ static int symbol_in_range(struct sym_entry *s, struct addr_range *ranges,
 	return 0;
 }
 
-static int symbol_valid(struct sym_entry *s)
+static const char *skip_prefix(const char *sym)
 {
+	if (symbol_prefix_char && *sym == symbol_prefix_char)
+		return sym + 1;
+	return sym;
+}

scripts/kallsyms.c: In function ‘symbol_valid’:
scripts/kallsyms.c:241:11: warning: assignment discards ‘const’ qualifier from pointer target type

  sym_name = skip_prefix(sym_name);
           ^
scripts/kallsyms.c:277:20: warning: assignment discards ‘const’ qualifier from pointer target type
    sym_name_before = skip_prefix(sym_name_before);
                    ^
scripts/kallsyms.c:282:19: warning: assignment discards ‘const’ qualifier from pointer target type
    sym_name_after = skip_prefix(sym_name_after);
                   ^

That sym_name should be const?

Quoted 25 lines

scripts/kallsyms.c +11 -3

+
+static int match_sys(const char *sym, const char *sys)
+{
+	return !strncmp(sys, "sys_", 4) && !strcmp(sym + 4, sys + 4);
+}
+
+static int symbol_valid(int idx)
+{
+	struct sym_entry *s = &table[idx];
+
 	/* Symbols which vary between passes.  Passes 1 and 2 must have
 	 * identical symbol lists.  The kallsyms_* symbols below are only added
 	 * after pass 1, they would be included in pass 2 when --all-symbols is

@@ -224,9 +238,7 @@ static int symbol_valid(struct sym_entry *s)
 	if (s->addr < kernel_start_addr)
 		return 0;
 
-	/* skip prefix char */
-	if (symbol_prefix_char && *sym_name == symbol_prefix_char)
-		sym_name++;
+	sym_name = skip_prefix(sym_name);
 
 
 	/* if --all-symbols is not specified, then symbols outside the text

scripts/kallsyms.c static int symbol_valid(struct sym_entry *s) · line 271

Quoted 5 lines

scripts/kallsyms.c +2 -0

@@ -255,6 +267,27 @@ static int symbol_valid(struct sym_entry *s)
 		if (strcmp(sym_name, special_symbols[i]) == 0)
 			return 0;
 
+	/* Ignore SyS_* alias system calls */
+	if (!strncmp(sym_name, "SyS_", 4)) {

I guess we won't have to hide more symbols from kallsyms. If we do, then probably will have to generalize this... Oh well.

Other than that, I think this a step in the right direction as the first patch makes the SyS_ symbols local and that's a good way for tools parsing the symbol table to know which symbol to show:

readelf -a vmlinux | grep -iE "\Wsys_" | sort -k8 | head -50
 77462: ffffffff815928f0    18 FUNC    GLOBAL DEFAULT    1 sys_accept
 43024: ffffffff815928f0    18 FUNC    LOCAL  DEFAULT    1 SyS_accept
 71564: ffffffff815926e0   526 FUNC    GLOBAL DEFAULT    1 sys_accept4
 43023: ffffffff815926e0   526 FUNC    LOCAL  DEFAULT    1 SyS_accept4
 50484: ffffffff81181170    26 FUNC    GLOBAL DEFAULT    1 sys_access
 12656: ffffffff81181170    26 FUNC    LOCAL  DEFAULT    1 SyS_access

...

i.e., the global one, provided all the other attributes are the same.

Before that we had them all identical:

readelf -a vmlinux | grep -iE "\Wsys_" | sort -k8 | head -50
 77446: ffffffff815928f0    18 FUNC    GLOBAL DEFAULT    1 sys_accept
 69532: ffffffff815928f0    18 FUNC    GLOBAL DEFAULT    1 SyS_accept
 71484: ffffffff815926e0   526 FUNC    GLOBAL DEFAULT    1 sys_accept4
 50916: ffffffff815926e0   526 FUNC    GLOBAL DEFAULT    1 SyS_accept4
 50192: ffffffff81181170    26 FUNC    GLOBAL DEFAULT    1 sys_access
 59364: ffffffff81181170    26 FUNC    GLOBAL DEFAULT    1 SyS_access
 52487: ffffffff81078920    18 FUNC    WEAK   DEFAULT    1 sys_acct
 57730: ffffffff812b7d20   629 FUNC    GLOBAL DEFAULT    1 sys_add_key
 64564: ffffffff812b7d20   629 FUNC    GLOBAL DEFAULT    1 SyS_add_key
 74229: ffffffff810c7b00   154 FUNC    GLOBAL DEFAULT    1 sys_adjtimex
 50534: ffffffff810c7b00   154 FUNC    GLOBAL DEFAULT    1 SyS_adjtimex
 58974: ffffffff810cbab0    18 FUNC    GLOBAL DEFAULT    1 sys_alarm
 54082: ffffffff810cbab0    18 FUNC    GLOBAL DEFAULT    1 SyS_alarm

...

Signature · 5 lines

Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Steven Rostedt

author

2015-05-22 12:42 UTC #4 lore raw

Re: [RFC][PATCH 2/2] kallsyms: Do not display SyS_foo() syscall aliases in kallsyms

Quoted 27 lines · On Fri, 22 May 2015 12:55:49 +0200 Borislav Petkov <[email protected]> wrote:

Quoted 5 lines

scripts/kallsyms.c +0 -0
index 8fa81e84e295..a64d89c6641c 100644

scripts/kallsyms.c static int symbol_in_range(struct sym_entry *s, struct addr_range *ranges, · line 201

Quoted 10 lines

scripts/kallsyms.c +5 -1

@@ -193,8 +193,22 @@ static int symbol_in_range(struct sym_entry *s, struct addr_range *ranges,
 	return 0;
 }
 
-static int symbol_valid(struct sym_entry *s)
+static const char *skip_prefix(const char *sym)
 {
+	if (symbol_prefix_char && *sym == symbol_prefix_char)
+		return sym + 1;
+	return sym;
+}

scripts/kallsyms.c: In function ‘symbol_valid’:
scripts/kallsyms.c:241:11: warning: assignment discards ‘const’ qualifier from pointer target type

  sym_name = skip_prefix(sym_name);
           ^
scripts/kallsyms.c:277:20: warning: assignment discards ‘const’ qualifier from pointer target type
    sym_name_before = skip_prefix(sym_name_before);
                    ^
scripts/kallsyms.c:282:19: warning: assignment discards ‘const’ qualifier from pointer target type
    sym_name_after = skip_prefix(sym_name_after);
                   ^

Quoted 1 line

That sym_name should be const?

Hmm, thanks. I'll take a look at this.

Quoted 29 lines

scripts/kallsyms.c +11 -3

+
+static int match_sys(const char *sym, const char *sys)
+{
+	return !strncmp(sys, "sys_", 4) && !strcmp(sym + 4, sys + 4);
+}
+
+static int symbol_valid(int idx)
+{
+	struct sym_entry *s = &table[idx];
+
 	/* Symbols which vary between passes.  Passes 1 and 2 must have
 	 * identical symbol lists.  The kallsyms_* symbols below are only added
 	 * after pass 1, they would be included in pass 2 when --all-symbols is

@@ -224,9 +238,7 @@ static int symbol_valid(struct sym_entry *s)
 	if (s->addr < kernel_start_addr)
 		return 0;
 
-	/* skip prefix char */
-	if (symbol_prefix_char && *sym_name == symbol_prefix_char)
-		sym_name++;
+	sym_name = skip_prefix(sym_name);
 
 
 	/* if --all-symbols is not specified, then symbols outside the text

@@ -255,6 +267,27 @@ static int symbol_valid(struct sym_entry *s)
 		if (strcmp(sym_name, special_symbols[i]) == 0)
 			return 0;

Quoted 5 lines

scripts/kallsyms.c static int symbol_valid(struct sym_entry *s) · line 271
Quoted 2 lines
scripts/kallsyms.c +2 -0
+	/* Ignore SyS_* alias system calls */
+	if (!strncmp(sym_name, "SyS_", 4)) {
I guess we won't have to hide more symbols from kallsyms. If we do, then probably will have to generalize this... Oh well.

I started writing a more generalize version to begin with, then realized, "why? this isn't something we do every day". Yeah, if we add another one then we should take the time to do so.

Quoted 12 lines

Other than that, I think this a step in the right direction as the first patch makes the SyS_ symbols local and that's a good way for tools parsing the symbol table to know which symbol to show:

readelf -a vmlinux | grep -iE "\Wsys_" | sort -k8 | head -50
 77462: ffffffff815928f0    18 FUNC    GLOBAL DEFAULT    1 sys_accept
 43024: ffffffff815928f0    18 FUNC    LOCAL  DEFAULT    1 SyS_accept
 71564: ffffffff815926e0   526 FUNC    GLOBAL DEFAULT    1 sys_accept4
 43023: ffffffff815926e0   526 FUNC    LOCAL  DEFAULT    1 SyS_accept4
 50484: ffffffff81181170    26 FUNC    GLOBAL DEFAULT    1 sys_access
 12656: ffffffff81181170    26 FUNC    LOCAL  DEFAULT    1 SyS_access

Quoted 5 lines

...
i.e., the global one, provided all the other attributes are the same.
Before that we had them all identical:

Yep!

Thanks, I'll update this patch.

-- Steve

Quoted 17 lines

readelf -a vmlinux | grep -iE "\Wsys_" | sort -k8 | head -50
 77446: ffffffff815928f0    18 FUNC    GLOBAL DEFAULT    1 sys_accept
 69532: ffffffff815928f0    18 FUNC    GLOBAL DEFAULT    1 SyS_accept
 71484: ffffffff815926e0   526 FUNC    GLOBAL DEFAULT    1 sys_accept4
 50916: ffffffff815926e0   526 FUNC    GLOBAL DEFAULT    1 SyS_accept4
 50192: ffffffff81181170    26 FUNC    GLOBAL DEFAULT    1 sys_access
 59364: ffffffff81181170    26 FUNC    GLOBAL DEFAULT    1 SyS_access
 52487: ffffffff81078920    18 FUNC    WEAK   DEFAULT    1 sys_acct
 57730: ffffffff812b7d20   629 FUNC    GLOBAL DEFAULT    1 sys_add_key
 64564: ffffffff812b7d20   629 FUNC    GLOBAL DEFAULT    1 SyS_add_key
 74229: ffffffff810c7b00   154 FUNC    GLOBAL DEFAULT    1 sys_adjtimex
 50534: ffffffff810c7b00   154 FUNC    GLOBAL DEFAULT    1 SyS_adjtimex
 58974: ffffffff810cbab0    18 FUNC    GLOBAL DEFAULT    1 sys_alarm
 54082: ffffffff810cbab0    18 FUNC    GLOBAL DEFAULT    1 SyS_alarm

...

Pratyush Anand

2017-06-15 14:27 UTC #5 lore raw

RFC 2/2 kallsyms: Do not display SyS_foo() syscall aliases in kallsyms

In reply to Steven Rostedt

From: Steven Rostedt <[email protected]>

Hi Steve,

Is there any plan for next revision of this patchset.

Regards
Pratyush

Steven Rostedt

author

2017-06-15 14:30 UTC #6 lore raw

Re: [RFC, 2/2] kallsyms: Do not display SyS_foo() syscall aliases in kallsyms

Quoted 7 lines · On Thu, 15 Jun 2017 19:56:40 +0530 Pratyush Anand <[email protected]> wrote:

From: Steven Rostedt <[email protected]>
Hi Steve,
Is there any plan for next revision of this patchset.

Heh, I forgot about this. Actually, I didn't. I was just thinking about the issue of SyS vs sys, but forgot I had patches to fix it.

I should push it again.

Thanks for the reminder.

-- Steve

Pratyush Anand

2017-06-15 14:37 UTC #7 lore raw

Re: [RFC, 2/2] kallsyms: Do not display SyS_foo() syscall aliases in kallsyms

Quoted 11 lines · Steven Rostedt

Quoted 7 lines · On Thu, 15 Jun 2017 19:56:40 +0530 Pratyush Anand <[email protected]> wrote:
From: Steven Rostedt <[email protected]>
Hi Steve,
Is there any plan for next revision of this patchset.

Quoted 6 lines

Heh, I forgot about this. Actually, I didn't. I was just thinking about the issue of SyS vs sys, but forgot I had patches to fix it.
I should push it again.
Thanks for the reminder.

Thanks for the quick reply :-)

Pratyush

Pratyush Anand

2017-06-15 14:36 UTC #8 lore raw

Re: [RFC, 2/2] kallsyms: Do not display SyS_foo() syscall aliases in kallsyms

In reply to Pratyush Anand

Quoted 1 line · Pratyush Anand

From: Steven Rostedt <[email protected]>

Sorry, Please ignore this. I did not had this patch in my inbox and replied through patchwork mailbox. Forgot to change the "from" field. Here is the patch link, which could resolve issues with trace filtering of aliased system calls.

https://patchwork.kernel.org/patch/6351951/

Please let me know if I can help in working for next revision.

Pratyush

Quoted 9 lines

Hi Steve,
Is there any plan for next revision of this patchset.
Regards
Pratyush