linux-kernel - Re: [PATCH 0001/0001] format idle IP output func+offset/length

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:   Mon, 6 Nov 2017 07:11:47 -0600
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     "Liu, Changcheng" <changcheng.liu@...el.com>
Cc:     Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
        Petr Mladek <pmladek@...e.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0001/0001] format idle IP output func+offset/length

On Mon, Nov 06, 2017 at 06:52:03PM +0800, Liu, Changcheng wrote:
> kaslr feature is enabled in kernel.
> Remove kernel text address when dumping idle IP info
> 
> Signed-off-by: Liu Changcheng <changcheng.liu@...el.com>
> Signed-off-by: Jerry Liu <primerlink@...il.com>
> 
> diff --git a/lib/nmi_backtrace.c b/lib/nmi_backtrace.c
> index 0bc0a35..9cc4178 100644
> --- a/lib/nmi_backtrace.c
> +++ b/lib/nmi_backtrace.c
> @@ -92,7 +92,7 @@ bool nmi_cpu_backtrace(struct pt_regs *regs)
>  	if (cpumask_test_cpu(cpu, to_cpumask(backtrace_mask))) {
>  		arch_spin_lock(&lock);
>  		if (regs && cpu_in_idle(instruction_pointer(regs))) {
> -			pr_warn("NMI backtrace for cpu %d skipped: idling at pc %#lx\n",
> +			pr_warn("NMI backtrace for cpu %d skipped: idling at %pS\n",
>  				cpu, instruction_pointer(regs));
>  		} else {
>  			pr_warn("NMI backtrace for cpu %d\n", cpu);

1) The patch introduces a compile warning.

2) When posting a new version of the patch, it should have a new version
   in the subject, e.g. "PATCH v2".

3) The subject is missing a prefix, like "lib_backtrace: "

4) The subject isn't very clear, how about something like:

   lib_backtrace: fix kernel text address leak

5) The description isn't correct.  KASLR isn't always enabled, it
   depends on the user's config.  But even without KASLR, we don't want
   to leak kernel text addresses.

6) The description should use complete sentences.

7) I'm not sure the "Signed-off-by:" chain is correct.  Was Jerry Liu
   the original author of the patch?

-- 
Josh