[RFC PATCH] kernel/kallsyms.c: only show legal kernel symbol

[RFC PATCH] kernel/kallsyms.c: only show legal kernel symbol

[Ming Lei]

Address of non-module kernel symbol should always be located
from CONFIG_PAGE_OFFSET on, so only show these legal kernel
symbols in /proc/kallsyms.

On ARM, some symbols(see below) may drop in relocatable code, so
perf can't parse kernel symbols any more from /proc/kallsyms, this
patch fixes the problem.

	00000000 t __vectors_start
	00000020 A cpu_v7_suspend_size
	00001000 t __stubs_start
	00001004 t vector_rst
	00001020 t vector_irq
	000010a0 t vector_dabt
	00001120 t vector_pabt
	000011a0 t vector_und
	00001220 t vector_addrexcptn
	00001224 t vector_fiq
	00001224 T vector_fiq_offset

The issue can be fixed in scripts/kallsyms.c too, but looks this
approach is easier.

Cc: Russell King <linux@arm.linux.org.uk>
Cc: linux-arm-kernel at lists.infradead.org
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Chen Gang <gang.chen@asianux.com>
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 kernel/kallsyms.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 3127ad5..184f271 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -543,7 +543,7 @@ static int s_show(struct seq_file *m, void *p)
 					tolower(iter->type);
 		seq_printf(m, "%pK %c %s\t[%s]\n", (void *)iter->value,
 			   type, iter->name, iter->module_name);
-	} else
+	} else if (iter->value >= CONFIG_PAGE_OFFSET)
 		seq_printf(m, "%pK %c %s\n", (void *)iter->value,
 			   iter->type, iter->name);
 	return 0;
-- 
1.7.9.5

[RFC PATCH] kernel/kallsyms.c: only show legal kernel symbol

[Rusty Russell]

Ming Lei <tom.leiming@gmail.com> writes:
> Address of non-module kernel symbol should always be located
> from CONFIG_PAGE_OFFSET on, so only show these legal kernel
> symbols in /proc/kallsyms.
>
> On ARM, some symbols(see below) may drop in relocatable code, so
> perf can't parse kernel symbols any more from /proc/kallsyms, this
> patch fixes the problem.
>
> 	00000000 t __vectors_start
> 	00000020 A cpu_v7_suspend_size
> 	00001000 t __stubs_start
> 	00001004 t vector_rst
> 	00001020 t vector_irq
> 	000010a0 t vector_dabt
> 	00001120 t vector_pabt
> 	000011a0 t vector_und
> 	00001220 t vector_addrexcptn
> 	00001224 t vector_fiq
> 	00001224 T vector_fiq_offset
>
> The issue can be fixed in scripts/kallsyms.c too, but looks this
> approach is easier.

This fix looks hacky; if these symbols are not available, don't just
remove them from /proc/kallsyms, but don't put them in the kernel at
all.

That way, they won't interfere with other kallsyms uses (eg. backtrace).

Or, better, figure out a smart way of excluding them by knowing why
these symbol addresses are wrong.

Thanks,
Rusty.

> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: Chen Gang <gang.chen@asianux.com>
> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
> ---
>  kernel/kallsyms.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
> index 3127ad5..184f271 100644
> --- a/kernel/kallsyms.c
> +++ b/kernel/kallsyms.c
> @@ -543,7 +543,7 @@ static int s_show(struct seq_file *m, void *p)
>  					tolower(iter->type);
>  		seq_printf(m, "%pK %c %s\t[%s]\n", (void *)iter->value,
>  			   type, iter->name, iter->module_name);
> -	} else
> +	} else if (iter->value >= CONFIG_PAGE_OFFSET)
>  		seq_printf(m, "%pK %c %s\n", (void *)iter->value,
>  			   iter->type, iter->name);
>  	return 0;
> -- 
> 1.7.9.5

[RFC PATCH] kernel/kallsyms.c: only show legal kernel symbol

[Ming Lei]

On Thu, Oct 24, 2013 at 9:21 AM, Rusty Russell <rusty@rustcorp.com.au> wrote:
> Ming Lei <tom.leiming@gmail.com> writes:
>> Address of non-module kernel symbol should always be located
>> from CONFIG_PAGE_OFFSET on, so only show these legal kernel
>> symbols in /proc/kallsyms.
>>
>> On ARM, some symbols(see below) may drop in relocatable code, so
>> perf can't parse kernel symbols any more from /proc/kallsyms, this
>> patch fixes the problem.
>>
>>       00000000 t __vectors_start
>>       00000020 A cpu_v7_suspend_size
>>       00001000 t __stubs_start
>>       00001004 t vector_rst
>>       00001020 t vector_irq
>>       000010a0 t vector_dabt
>>       00001120 t vector_pabt
>>       000011a0 t vector_und
>>       00001220 t vector_addrexcptn
>>       00001224 t vector_fiq
>>       00001224 T vector_fiq_offset
>>
>> The issue can be fixed in scripts/kallsyms.c too, but looks this
>> approach is easier.
>
> This fix looks hacky; if these symbols are not available, don't just
> remove them from /proc/kallsyms, but don't put them in the kernel at
> all.
>
> That way, they won't interfere with other kallsyms uses (eg. backtrace).

Yes, I agree.

> Or, better, figure out a smart way of excluding them by knowing why
> these symbol addresses are wrong.

Actually the problem is caused by commit b9b32bf70f2(ARM: use linker
magic for vectors and vector stubs), maybe Russell can figure out a smart
way to exclude these symbols.


Thanks,
-- 
Ming Lei

[RFC PATCH] kernel/kallsyms.c: only show legal kernel symbol

[Russell King - ARM Linux]

On Thu, Oct 24, 2013 at 11:51:18AM +1030, Rusty Russell wrote:
> Ming Lei <tom.leiming@gmail.com> writes:
> > Address of non-module kernel symbol should always be located
> > from CONFIG_PAGE_OFFSET on, so only show these legal kernel
> > symbols in /proc/kallsyms.
> >
> > On ARM, some symbols(see below) may drop in relocatable code, so
> > perf can't parse kernel symbols any more from /proc/kallsyms, this
> > patch fixes the problem.
> >
> > 	00000000 t __vectors_start
> > 	00000020 A cpu_v7_suspend_size
> > 	00001000 t __stubs_start
> > 	00001004 t vector_rst
> > 	00001020 t vector_irq
> > 	000010a0 t vector_dabt
> > 	00001120 t vector_pabt
> > 	000011a0 t vector_und
> > 	00001220 t vector_addrexcptn
> > 	00001224 t vector_fiq
> > 	00001224 T vector_fiq_offset
> >
> > The issue can be fixed in scripts/kallsyms.c too, but looks this
> > approach is easier.
> 
> This fix looks hacky; if these symbols are not available, don't just
> remove them from /proc/kallsyms, but don't put them in the kernel at
> all.

How do you "don't put them in the kernel at all" when they're used by
the kernel internally as offsets?

If you mean, just get rid of them, shall I just add these as magic
numbers instead based on the values in this email?  Is that really a
sane solution?

No, we have to keep these symbols IMHO.

[RFC PATCH] kernel/kallsyms.c: only show legal kernel symbol

[Ming Lei]

On Thu, Oct 24, 2013 at 4:45 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Thu, Oct 24, 2013 at 11:51:18AM +1030, Rusty Russell wrote:
>> Ming Lei <tom.leiming@gmail.com> writes:
>> > Address of non-module kernel symbol should always be located
>> > from CONFIG_PAGE_OFFSET on, so only show these legal kernel
>> > symbols in /proc/kallsyms.
>> >
>> > On ARM, some symbols(see below) may drop in relocatable code, so
>> > perf can't parse kernel symbols any more from /proc/kallsyms, this
>> > patch fixes the problem.
>> >
>> >     00000000 t __vectors_start
>> >     00000020 A cpu_v7_suspend_size
>> >     00001000 t __stubs_start
>> >     00001004 t vector_rst
>> >     00001020 t vector_irq
>> >     000010a0 t vector_dabt
>> >     00001120 t vector_pabt
>> >     000011a0 t vector_und
>> >     00001220 t vector_addrexcptn
>> >     00001224 t vector_fiq
>> >     00001224 T vector_fiq_offset
>> >
>> > The issue can be fixed in scripts/kallsyms.c too, but looks this
>> > approach is easier.
>>
>> This fix looks hacky; if these symbols are not available, don't just
>> remove them from /proc/kallsyms, but don't put them in the kernel at
>> all.
>
> How do you "don't put them in the kernel at all" when they're used by
> the kernel internally as offsets?
>
> If you mean, just get rid of them, shall I just add these as magic
> numbers instead based on the values in this email?  Is that really a
> sane solution?
>
> No, we have to keep these symbols IMHO.

If so, looks we have to hide them to userspace, so the patch
should be OK because the approach is correct and no obvious
side-effect.


Thanks,
-- 
Ming Lei

[RFC PATCH] kernel/kallsyms.c: only show legal kernel symbol

[Rusty Russell]

Russell King - ARM Linux <linux@arm.linux.org.uk> writes:
> On Thu, Oct 24, 2013 at 11:51:18AM +1030, Rusty Russell wrote:
>> Ming Lei <tom.leiming@gmail.com> writes:
>> > Address of non-module kernel symbol should always be located
>> > from CONFIG_PAGE_OFFSET on, so only show these legal kernel
>> > symbols in /proc/kallsyms.
>> >
>> > On ARM, some symbols(see below) may drop in relocatable code, so
>> > perf can't parse kernel symbols any more from /proc/kallsyms, this
>> > patch fixes the problem.
>> >
>> > 	00000000 t __vectors_start
>> > 	00000020 A cpu_v7_suspend_size
>> > 	00001000 t __stubs_start
>> > 	00001004 t vector_rst
>> > 	00001020 t vector_irq
>> > 	000010a0 t vector_dabt
>> > 	00001120 t vector_pabt
>> > 	000011a0 t vector_und
>> > 	00001220 t vector_addrexcptn
>> > 	00001224 t vector_fiq
>> > 	00001224 T vector_fiq_offset
>> >
>> > The issue can be fixed in scripts/kallsyms.c too, but looks this
>> > approach is easier.
>> 
>> This fix looks hacky; if these symbols are not available, don't just
>> remove them from /proc/kallsyms, but don't put them in the kernel at
>> all.
>
> How do you "don't put them in the kernel at all" when they're used by
> the kernel internally as offsets?

Sorry, I was imprecise.  I was referring to the kernel's kallsyms
tables produced by scripts/kallsyms.c.  This patch left them in the
the kallsyms tables and filtered them out from /proc/kallsyms.

It's weird that cpu_v7_suspend_size appeared above, since kallsyms
should filter out 'A' symbols already.

> If you mean, just get rid of them, shall I just add these as magic
> numbers instead based on the values in this email?  Is that really a
> sane solution?
>
> No, we have to keep these symbols IMHO.

Can you make them absolute symbols?  That should Just Work for kallsyms.

Cheers,
Rusty.

[RFC PATCH] kernel/kallsyms.c: only show legal kernel symbol

[Ming Lei]

On Fri, Oct 25, 2013 at 7:08 AM, Rusty Russell <rusty@rustcorp.com.au> wrote:
>
> Sorry, I was imprecise.  I was referring to the kernel's kallsyms
> tables produced by scripts/kallsyms.c.  This patch left them in the
> the kallsyms tables and filtered them out from /proc/kallsyms.

Yes, but it isn't easy to do it by script/kallsyms.c , and IMO, it should
be correct to hide them for user space but keep them in kallsyms table.

>
> It's weird that cpu_v7_suspend_size appeared above, since kallsyms
> should filter out 'A' symbols already.

Sorry, the 'A' symbol is a mistake, but the others do exist in /proc/kallsyms.


Thanks,
-- 
Ming Lei

[RFC PATCH] kernel/kallsyms.c: only show legal kernel symbol

[Rusty Russell]

Ming Lei <tom.leiming@gmail.com> writes:
> On Fri, Oct 25, 2013 at 7:08 AM, Rusty Russell <rusty@rustcorp.com.au> wrote:
>>
>> Sorry, I was imprecise.  I was referring to the kernel's kallsyms
>> tables produced by scripts/kallsyms.c.  This patch left them in the
>> the kallsyms tables and filtered them out from /proc/kallsyms.
>
> Yes, but it isn't easy to do it by script/kallsyms.c , and IMO, it should
> be correct to hide them for user space but keep them in kallsyms table.

So they'll appear in backtraces?  And turn up randomly for other symbol
dereferences?

I don't think you really want this!

>> It's weird that cpu_v7_suspend_size appeared above, since kallsyms
>> should filter out 'A' symbols already.
>
> Sorry, the 'A' symbol is a mistake, but the others do exist in /proc/kallsyms.

Ah, OK.

Cheers,
Rusty.

[RFC PATCH] kernel/kallsyms.c: only show legal kernel symbol

[Ming Lei]

On Fri, Oct 25, 2013 at 1:50 PM, Rusty Russell <rusty@rustcorp.com.au> wrote:
> Ming Lei <tom.leiming@gmail.com> writes:
>> On Fri, Oct 25, 2013 at 7:08 AM, Rusty Russell <rusty@rustcorp.com.au> wrote:
>>>
>>> Sorry, I was imprecise.  I was referring to the kernel's kallsyms
>>> tables produced by scripts/kallsyms.c.  This patch left them in the
>>> the kallsyms tables and filtered them out from /proc/kallsyms.
>>
>> Yes, but it isn't easy to do it by script/kallsyms.c , and IMO, it should
>> be correct to hide them for user space but keep them in kallsyms table.
>
> So they'll appear in backtraces?  And turn up randomly for other symbol
> dereferences?
>
> I don't think you really want this!

Basically these symbols are only used to generate code, and in
kernel mode, CPU won't run into the corresponding addresses
because the generate code is copied to other address during booting,
so I understand they won't appear in backtraces.


Thanks,
-- 
Ming Lei

[RFC PATCH] kernel/kallsyms.c: only show legal kernel symbol

[Rusty Russell]

Ming Lei <tom.leiming@gmail.com> writes:
> On Fri, Oct 25, 2013 at 1:50 PM, Rusty Russell <rusty@rustcorp.com.au> wrote:
>> Ming Lei <tom.leiming@gmail.com> writes:
>>> On Fri, Oct 25, 2013 at 7:08 AM, Rusty Russell <rusty@rustcorp.com.au> wrote:
>>>>
>>>> Sorry, I was imprecise.  I was referring to the kernel's kallsyms
>>>> tables produced by scripts/kallsyms.c.  This patch left them in the
>>>> the kallsyms tables and filtered them out from /proc/kallsyms.
>>>
>>> Yes, but it isn't easy to do it by script/kallsyms.c , and IMO, it should
>>> be correct to hide them for user space but keep them in kallsyms table.
>>
>> So they'll appear in backtraces?  And turn up randomly for other symbol
>> dereferences?
>>
>> I don't think you really want this!
>
> Basically these symbols are only used to generate code, and in
> kernel mode, CPU won't run into the corresponding addresses
> because the generate code is copied to other address during booting,
> so I understand they won't appear in backtraces.

An oops occurs when something went *wrong*.  We look up all kinds of
stuff.  Are you so sure that *none* of the callers will ever see these
strange symbols and produce a confusing result?

Cheers,
Rusty.

[RFC PATCH] kernel/kallsyms.c: only show legal kernel symbol

[Ming Lei]

On Fri, Oct 25, 2013 at 7:58 PM, Rusty Russell <rusty@rustcorp.com.au> wrote:
>>
>> Basically these symbols are only used to generate code, and in
>> kernel mode, CPU won't run into the corresponding addresses
>> because the generate code is copied to other address during booting,
>> so I understand they won't appear in backtraces.
>
> An oops occurs when something went *wrong*.  We look up all kinds of
> stuff.  Are you so sure that *none* of the callers will ever see these
> strange symbols and produce a confusing result?

Suppose that might happen, kernel should be smart enough to know
that the address is not inside kernel address space and won't produce
confusing result, right?


Thanks,
-- 
Ming Lei

[RFC PATCH] kernel/kallsyms.c: only show legal kernel symbol

[Rusty Russell]

Ming Lei <tom.leiming@gmail.com> writes:
> On Fri, Oct 25, 2013 at 7:58 PM, Rusty Russell <rusty@rustcorp.com.au> wrote:
>>>
>>> Basically these symbols are only used to generate code, and in
>>> kernel mode, CPU won't run into the corresponding addresses
>>> because the generate code is copied to other address during booting,
>>> so I understand they won't appear in backtraces.
>>
>> An oops occurs when something went *wrong*.  We look up all kinds of
>> stuff.  Are you so sure that *none* of the callers will ever see these
>> strange symbols and produce a confusing result?
>
> Suppose that might happen, kernel should be smart enough to know
> that the address is not inside kernel address space and won't produce
> confusing result, right?

I don't know...  It would be your job, as the person making the change,
to find all the users of kallsyms and prove that.

This is why it is easier not to include incorrect values in the kernel's
kallsyms in the first place.

Hope that helps,
Rusty.

[RFC PATCH] kernel/kallsyms.c: only show legal kernel symbol

[Ming Lei]

On Mon, 28 Oct 2013 13:44:30 +1030
Rusty Russell <rusty@rustcorp.com.au> wrote:

> Ming Lei <tom.leiming@gmail.com> writes:
> 
> I don't know...  It would be your job, as the person making the change,
> to find all the users of kallsyms and prove that.
> 
> This is why it is easier not to include incorrect values in the kernel's
> kallsyms in the first place.

OK, thanks for your comment, and I figured out one way to do it in
scripts/kallsyms.c, could you comment on below patch?

--
>From 4327534dedfa60d208ac3e23db7556c243e1c7dc Mon Sep 17 00:00:00 2001
From: Ming Lei <tom.leiming@gmail.com>
Date: Mon, 28 Oct 2013 13:04:49 +0800
Subject: [PATCH] scripts/kallsyms: filter symbols not in kernel address space

This patch uses CONFIG_PAGE_OFFSET to filter symbols which
are not in kernel address space because these symbols are
generally for generating code purpose and can't be run at
kernel mode, so we needn't keep them in /proc/kallsyms.

For example, on ARM there are some symbols which may be
linked in relocatable code section, then perf can't parse
symbols any more from /proc/kallsyms, this patch fixes the
problem.

Cc: Russell King <linux@arm.linux.org.uk>
Cc: linux-arm-kernel at lists.infradead.org
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Michal Marek <mmarek@suse.cz>
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 scripts/kallsyms.c      |   12 +++++++++++-
 scripts/link-vmlinux.sh |    2 ++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 487ac6f..9a11f9f 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -55,6 +55,7 @@ static struct sym_entry *table;
 static unsigned int table_size, table_cnt;
 static int all_symbols = 0;
 static char symbol_prefix_char = '\0';
+static unsigned long long kernel_start_addr = 0;
 
 int token_profit[0x10000];
 
@@ -65,7 +66,10 @@ unsigned char best_table_len[256];
 
 static void usage(void)
 {
-	fprintf(stderr, "Usage: kallsyms [--all-symbols] [--symbol-prefix=<prefix char>] < in.map > out.S\n");
+	fprintf(stderr, "Usage: kallsyms [--all-symbols] "
+			"[--symbol-prefix=<prefix char>] "
+			"[--page-offset=<CONFIG_PAGE_OFFSET>] "
+			"< in.map > out.S\n");
 	exit(1);
 }
 
@@ -194,6 +198,9 @@ static int symbol_valid(struct sym_entry *s)
 	int i;
 	int offset = 1;
 
+	if (s->addr < kernel_start_addr)
+		return 0;
+
 	/* skip prefix char */
 	if (symbol_prefix_char && *(s->sym + 1) == symbol_prefix_char)
 		offset++;
@@ -646,6 +653,9 @@ int main(int argc, char **argv)
 				if ((*p == '"' && *(p+2) == '"') || (*p == '\'' && *(p+2) == '\''))
 					p++;
 				symbol_prefix_char = *p;
+			} else if (strncmp(argv[i], "--page-offset=", 14) == 0) {
+				const char *p = &argv[i][14];
+				kernel_start_addr = strtoull(p, NULL, 16);
 			} else
 				usage();
 		}
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 0149949..32b10f5 100644
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -82,6 +82,8 @@ kallsyms()
 		kallsymopt="${kallsymopt} --all-symbols"
 	fi
 
+	kallsymopt="${kallsymopt} --page-offset=$CONFIG_PAGE_OFFSET"
+
 	local aflags="${KBUILD_AFLAGS} ${KBUILD_AFLAGS_KERNEL}               \
 		      ${NOSTDINC_FLAGS} ${LINUXINCLUDE} ${KBUILD_CPPFLAGS}"
 
-- 
1.7.9.5




Thanks,
-- 
Ming Lei

[RFC PATCH] kernel/kallsyms.c: only show legal kernel symbol

[Rusty Russell]

Ming Lei <tom.leiming@gmail.com> writes:
> On Mon, 28 Oct 2013 13:44:30 +1030
> Rusty Russell <rusty@rustcorp.com.au> wrote:
>
>> Ming Lei <tom.leiming@gmail.com> writes:
>> 
>> I don't know...  It would be your job, as the person making the change,
>> to find all the users of kallsyms and prove that.
>> 
>> This is why it is easier not to include incorrect values in the kernel's
>> kallsyms in the first place.
>
> OK, thanks for your comment, and I figured out one way to do it in
> scripts/kallsyms.c, could you comment on below patch?

Looks great! Seems like we spent more time arguing than it took you to
code that up.

Acked-by: Rusty Russell <rusty@rustcorp.com.au>

Russell, this seems logical for you to take along with the changes which
caused the problem?

Thanks,
Rusty.

> --
> From 4327534dedfa60d208ac3e23db7556c243e1c7dc Mon Sep 17 00:00:00 2001
> From: Ming Lei <tom.leiming@gmail.com>
> Date: Mon, 28 Oct 2013 13:04:49 +0800
> Subject: [PATCH] scripts/kallsyms: filter symbols not in kernel address space
>
> This patch uses CONFIG_PAGE_OFFSET to filter symbols which
> are not in kernel address space because these symbols are
> generally for generating code purpose and can't be run at
> kernel mode, so we needn't keep them in /proc/kallsyms.
>
> For example, on ARM there are some symbols which may be
> linked in relocatable code section, then perf can't parse
> symbols any more from /proc/kallsyms, this patch fixes the
> problem.
>
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: Michal Marek <mmarek@suse.cz>
> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
> ---
>  scripts/kallsyms.c      |   12 +++++++++++-
>  scripts/link-vmlinux.sh |    2 ++
>  2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
> index 487ac6f..9a11f9f 100644
> --- a/scripts/kallsyms.c
> +++ b/scripts/kallsyms.c
> @@ -55,6 +55,7 @@ static struct sym_entry *table;
>  static unsigned int table_size, table_cnt;
>  static int all_symbols = 0;
>  static char symbol_prefix_char = '\0';
> +static unsigned long long kernel_start_addr = 0;
>  
>  int token_profit[0x10000];
>  
> @@ -65,7 +66,10 @@ unsigned char best_table_len[256];
>  
>  static void usage(void)
>  {
> -	fprintf(stderr, "Usage: kallsyms [--all-symbols] [--symbol-prefix=<prefix char>] < in.map > out.S\n");
> +	fprintf(stderr, "Usage: kallsyms [--all-symbols] "
> +			"[--symbol-prefix=<prefix char>] "
> +			"[--page-offset=<CONFIG_PAGE_OFFSET>] "
> +			"< in.map > out.S\n");
>  	exit(1);
>  }
>  
> @@ -194,6 +198,9 @@ static int symbol_valid(struct sym_entry *s)
>  	int i;
>  	int offset = 1;
>  
> +	if (s->addr < kernel_start_addr)
> +		return 0;
> +
>  	/* skip prefix char */
>  	if (symbol_prefix_char && *(s->sym + 1) == symbol_prefix_char)
>  		offset++;
> @@ -646,6 +653,9 @@ int main(int argc, char **argv)
>  				if ((*p == '"' && *(p+2) == '"') || (*p == '\'' && *(p+2) == '\''))
>  					p++;
>  				symbol_prefix_char = *p;
> +			} else if (strncmp(argv[i], "--page-offset=", 14) == 0) {
> +				const char *p = &argv[i][14];
> +				kernel_start_addr = strtoull(p, NULL, 16);
>  			} else
>  				usage();
>  		}
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index 0149949..32b10f5 100644
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -82,6 +82,8 @@ kallsyms()
>  		kallsymopt="${kallsymopt} --all-symbols"
>  	fi
>  
> +	kallsymopt="${kallsymopt} --page-offset=$CONFIG_PAGE_OFFSET"
> +
>  	local aflags="${KBUILD_AFLAGS} ${KBUILD_AFLAGS_KERNEL}               \
>  		      ${NOSTDINC_FLAGS} ${LINUXINCLUDE} ${KBUILD_CPPFLAGS}"
>  
> -- 
> 1.7.9.5
>
>
>
>
> Thanks,
> -- 
> Ming Lei

[RFC PATCH] kernel/kallsyms.c: only show legal kernel symbol

[Russell King - ARM Linux]

On Mon, Oct 28, 2013 at 04:20:19PM +1030, Rusty Russell wrote:
> Ming Lei <tom.leiming@gmail.com> writes:
> > On Mon, 28 Oct 2013 13:44:30 +1030
> > Rusty Russell <rusty@rustcorp.com.au> wrote:
> >
> >> Ming Lei <tom.leiming@gmail.com> writes:
> >> 
> >> I don't know...  It would be your job, as the person making the change,
> >> to find all the users of kallsyms and prove that.
> >> 
> >> This is why it is easier not to include incorrect values in the kernel's
> >> kallsyms in the first place.
> >
> > OK, thanks for your comment, and I figured out one way to do it in
> > scripts/kallsyms.c, could you comment on below patch?
> 
> Looks great! Seems like we spent more time arguing than it took you to
> code that up.
> 
> Acked-by: Rusty Russell <rusty@rustcorp.com.au>
> 
> Russell, this seems logical for you to take along with the changes which
> caused the problem?

The changes are already in mainline since a long time (back in July/August
time).  Am I the right person to take stuff for scripts/ ?  Isn't that
more kbuild territory?

[RFC PATCH] kernel/kallsyms.c: only show legal kernel symbol

[Rusty Russell]

Russell King - ARM Linux <linux@arm.linux.org.uk> writes:
> On Mon, Oct 28, 2013 at 04:20:19PM +1030, Rusty Russell wrote:
>> Ming Lei <tom.leiming@gmail.com> writes:
>> > On Mon, 28 Oct 2013 13:44:30 +1030
>> > Rusty Russell <rusty@rustcorp.com.au> wrote:
>> >
>> >> Ming Lei <tom.leiming@gmail.com> writes:
>> >> 
>> >> I don't know...  It would be your job, as the person making the change,
>> >> to find all the users of kallsyms and prove that.
>> >> 
>> >> This is why it is easier not to include incorrect values in the kernel's
>> >> kallsyms in the first place.
>> >
>> > OK, thanks for your comment, and I figured out one way to do it in
>> > scripts/kallsyms.c, could you comment on below patch?
>> 
>> Looks great! Seems like we spent more time arguing than it took you to
>> code that up.
>> 
>> Acked-by: Rusty Russell <rusty@rustcorp.com.au>
>> 
>> Russell, this seems logical for you to take along with the changes which
>> caused the problem?
>
> The changes are already in mainline since a long time (back in July/August
> time).  Am I the right person to take stuff for scripts/ ?  Isn't that
> more kbuild territory?

Kallsyms tends to fall between modules and scripts.  I assume it's not
urgent, so no cc:stable on this one.

Applied,
Rusty.

[RFC PATCH] kernel/kallsyms.c: only show legal kernel symbol

[Ming Lei]

On Thu, Oct 31, 2013 at 11:14 AM, Rusty Russell <rusty@rustcorp.com.au> wrote:
> Russell King - ARM Linux <linux@arm.linux.org.uk> writes:
>
> Kallsyms tends to fall between modules and scripts.  I assume it's not
> urgent, so no cc:stable on this one.

Rusty, thanks a lot.

BTW, there is already other report on the problem:

   http://lkml.indiana.edu/hypermail/linux/kernel/1310.3/02772.html


Thanks,
-- 
Ming Lei

[RFC PATCH] kernel/kallsyms.c: only show legal kernel symbol

[Rusty Russell]

Ming Lei <tom.leiming@gmail.com> writes:

> On Thu, Oct 31, 2013 at 11:14 AM, Rusty Russell <rusty@rustcorp.com.au> wrote:
>> Russell King - ARM Linux <linux@arm.linux.org.uk> writes:
>>
>> Kallsyms tends to fall between modules and scripts.  I assume it's not
>> urgent, so no cc:stable on this one.
>
> Rusty, thanks a lot.
>
> BTW, there is already other report on the problem:
>
>    http://lkml.indiana.edu/hypermail/linux/kernel/1310.3/02772.html

OK, *that* references the commit which is the problem, when went into
v3.11 (b9b32bf70f2fb710b07c94e13afbc729afe221da)

Unfortunately, *that commit* was cc:stable, so this needs to be
cc:stable as well, not just >= 3.11.

Looking back on those patches, there's a mass of cc:stable on them.  The
descriptions are either misleading, or these patches prevent theoretical
attacks which means they shouldn't have been cc:stable.

Grr...
Rusty.