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

From: Rusty Russell
Date: Mon Oct 28 2013 - 01:51:01 EST


Ming Lei <tom.leiming@xxxxxxxxx> writes:
> On Mon, 28 Oct 2013 13:44:30 +1030
> Rusty Russell <rusty@xxxxxxxxxxxxxxx> wrote:
>
>> Ming Lei <tom.leiming@xxxxxxxxx> 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@xxxxxxxxxxxxxxx>

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@xxxxxxxxx>
> 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@xxxxxxxxxxxxxxxx>
> Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> Cc: Rusty Russell <rusty@xxxxxxxxxxxxxxx>
> Cc: Michal Marek <mmarek@xxxxxxx>
> Signed-off-by: Ming Lei <tom.leiming@xxxxxxxxx>
> ---
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/