linux-kernel - Re: [PATCH 0/5] [RFC] printk/ia64/ppc64/parisc64: let's deprecate %pF/%pf printk specifiers

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:   Tue, 19 Sep 2017 11:05:38 +0900
From:   Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
To:     Helge Deller <deller@....de>
Cc:     Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        "Luck, Tony" <tony.luck@...el.com>,
        Fenghua Yu <fenghua.yu@...el.com>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Paul Mackerras <paulus@...ba.org>,
        Michael Ellerman <mpe@...erman.id.au>,
        "James E . J . Bottomley" <jejb@...isc-linux.org>,
        Petr Mladek <pmladek@...e.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Jessica Yu <jeyu@...nel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        linux-ia64@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/5] [RFC] printk/ia64/ppc64/parisc64: let's deprecate
 %pF/%pf printk specifiers

On (09/18/17 20:39), Helge Deller wrote:
[..]
> > A few new warnings when building on ia64:
> > 
> > arch/ia64/kernel/module.c:931: warning: passing argument 1 of 'dereference_function_descriptor' makes pointer from integer without a cast
> > arch/ia64/kernel/module.c:931: warning: return makes integer from pointer without a cast
> > kernel/kallsyms.c:325: warning: assignment makes integer from pointer without a cast
> > kernel/kallsyms.c:325: warning: passing argument 1 of 'dereference_kernel_function_descriptor' makes pointer from integer without a cast
> 
> 
> I got similiar warnings on parisc.
> This patch on top of yours fixed those:
> 

Tony, Helge,

thanks for the reports!

I'll simply convert everything to `unsigned long'. including the
dereference_function_descriptor() function [I believe there are
still some casts happening when we pass addr from kernel/module
dereference functions to dereference_function_descriptor(), or
when we return `void *' back to symbol resolution code, etc.)
besides, it seems that everything that uses
dereference_function_descriptor() wants `unsigned long' anyway:

drivers/misc/kgdbts.c:  addr = (unsigned long) dereference_function_descriptor((void *)addr);
init/main.c:    addr = (unsigned long) dereference_function_descriptor(fn);
kernel/extable.c:       addr = (unsigned long) dereference_function_descriptor(ptr);
kernel/module.c:        unsigned long a = (unsigned long)dereference_function_descriptor(addr);

so I'll just switch it to ulong.


> I did tried your testcases too.
> 
> "echo 1 > /proc/sys/vm/drop_caches" gave correct output:
>  printk#1 schedule_timeout+0x0/0x4a8
>  printk#2 schedule_timeout+0x0/0x4a8
>  printk#3 proc_sys_call_handler+0x120/0x180
>  printk#4 proc_sys_call_handler+0x120/0x180
>  printk#5 proc_sys_call_handler+0x120/0x180
>  printk#6 proc_sys_call_handler+0x120/0x180
> 
> and here is "modprobe zram":
>  printk#7 __UNIQUE_ID_vermagic8+0xb9a4/0xbd04 [zram]
>  printk#8 __UNIQUE_ID_vermagic8+0xb9a4/0xbd04 [zram]
>  printk#9 do_one_initcall+0x194/0x290
>  printk#10 do_one_initcall+0x194/0x290
>  printk#11 do_one_initcall+0x194/0x290
>  printk#12 do_one_initcall+0x194/0x290
>  printk#13 zram_init+0x22c/0x2a0 [zram]
>  printk#14 zram_init+0x22c/0x2a0 [zram]
>  printk#15 zram_init+0x22c/0x2a0 [zram]
>  printk#16 zram_init+0x22c/0x2a0 [zram]
> 
> I wonder why printk#7 and printk#8 don't show "zram_init"...

interesting... what does the unpatched kernel show?


> Regarding your patches:
> 
> In arch/parisc/kernel/process.c:
> +void *dereference_kernel_function_descriptor(void *ptr)
> +{
> +       if (ptr < (void *)__start_opd || (void *)__end_opd < ptr)
> 
> This needs to be (__end_opd is outside):
> +       if (ptr < (void *)__start_opd || (void *)__end_opd <= ptr)
> 
> The same is true for the checks in the other arches.

um... yeah. __end_opd is definitely not a valid place for a descriptor!
I think I had `if (!(ptr >= __start_opd && ptr < __end_opd))' which I
wrongly converted. "shame, shame, shame".

thanks!


> I'd suggest to move the various
> 	extern char __start_opd[], __end_opd[];
> out of arch/<arch>/include/asm/sections.h and into <asm-generic/sections.h>

ok, will take a look.

	-ss