Re: [PATCH 0/2] introduce DUMP_PREFIX_UNHASHED for hex dumps

2021-01-27 Thread Vlastimil Babka
On 1/27/21 11:11 AM, Petr Mladek wrote: > On Tue 2021-01-26 12:40:32, Steven Rostedt wrote: >> On Tue, 26 Jan 2021 12:39:12 -0500 >> Steven Rostedt wrote: >> >> > On Tue, 26 Jan 2021 11:30:02 -0600 >> > Timur Tabi wrote: >> > >> > > On 1/26/21 11:14 AM, Vlastimil Babka wrote: >> > > > If it

Re: [PATCH 0/2] introduce DUMP_PREFIX_UNHASHED for hex dumps

2021-01-27 Thread Petr Mladek
On Tue 2021-01-26 12:40:32, Steven Rostedt wrote: > On Tue, 26 Jan 2021 12:39:12 -0500 > Steven Rostedt wrote: > > > On Tue, 26 Jan 2021 11:30:02 -0600 > > Timur Tabi wrote: > > > > > On 1/26/21 11:14 AM, Vlastimil Babka wrote: > > > > If it was a boot option, I would personally be for

Re: [PATCH 0/2] introduce DUMP_PREFIX_UNHASHED for hex dumps

2021-01-26 Thread Timur Tabi
On 1/26/21 10:47 AM, Vlastimil Babka wrote: Given Linus' current stance later in this thread, could we revive the idea of a boot time option, or at least a CONFIG (I assume a runtime toggle would be too much, even if limited to !kernel_lockdown:) , that would disable all hashing? It would be

Re: [PATCH 0/2] introduce DUMP_PREFIX_UNHASHED for hex dumps

2021-01-26 Thread Timur Tabi
On 1/26/21 8:11 PM, Sergey Senozhatsky wrote: +1 for boot param with a scary name and dmesg WARNING WARNING WARNING that should look even scarier. Timur, do you have time to take a look and submit a patch? Yes, I'll submit a patch.

Re: [PATCH 0/2] introduce DUMP_PREFIX_UNHASHED for hex dumps

2021-01-26 Thread Sergey Senozhatsky
On (21/01/26 20:29), John Ogness wrote: > > On 2021-01-26, Steven Rostedt wrote: > > And even if we make this a boot time option, perhaps we should still > > include that nasty dmesg notice, which will let people know that the > > kernel has unhashed values. > > +1 > > The notice would

Re: [PATCH 0/2] introduce DUMP_PREFIX_UNHASHED for hex dumps

2021-01-26 Thread John Ogness
On 2021-01-26, Steven Rostedt wrote: > And even if we make this a boot time option, perhaps we should still > include that nasty dmesg notice, which will let people know that the > kernel has unhashed values. +1 The notice would probably be the main motivation for distros/users to avoid

Re: [PATCH 0/2] introduce DUMP_PREFIX_UNHASHED for hex dumps

2021-01-26 Thread Steven Rostedt
On Tue, 26 Jan 2021 11:30:02 -0600 Timur Tabi wrote: > On 1/26/21 11:14 AM, Vlastimil Babka wrote: > > If it was a boot option, I would personally be for leaving hashing enabled > > by > > default, with opt-in boot option to disable it. > > A boot option would solve all my problems. I

Re: [PATCH 0/2] introduce DUMP_PREFIX_UNHASHED for hex dumps

2021-01-26 Thread Vlastimil Babka
On 1/26/21 5:59 PM, Timur Tabi wrote: > On 1/26/21 10:47 AM, Vlastimil Babka wrote: >> Given Linus' current stance later in this thread, could we revive the idea >> of a >> boot time option, or at least a CONFIG (I assume a runtime toggle would be >> too >> much, even if limited to

Re: [PATCH 0/2] introduce DUMP_PREFIX_UNHASHED for hex dumps

2021-01-26 Thread Steven Rostedt
On Tue, 26 Jan 2021 10:59:12 -0600 Timur Tabi wrote: > The only drawback to this idea is: what happens if distros start > enabling CONFIG_PRINTK_NEVER_HASH by default, just because it makes > debugging easier? I do believe distros should be more concerned about security than using this for

Re: [PATCH 0/2] introduce DUMP_PREFIX_UNHASHED for hex dumps

2021-01-26 Thread Vlastimil Babka
On 1/19/21 11:38 AM, Sergey Senozhatsky wrote: > On (21/01/19 01:47), Matthew Wilcox wrote: > [..] >> >> > So maybe DUMP_PREFIX_UNHASHED can do the unhashed dump only when >> > CONFIG_DEBUG_KERNEL=y and fallback to DUMP_PREFIX_ADDRESS otherwise? >> >> Distros enable CONFIG_DEBUG_KERNEL. > > Oh,

Re: [PATCH 0/2] introduce DUMP_PREFIX_UNHASHED for hex dumps

2021-01-26 Thread Steven Rostedt
On Tue, 26 Jan 2021 12:39:12 -0500 Steven Rostedt wrote: > On Tue, 26 Jan 2021 11:30:02 -0600 > Timur Tabi wrote: > > > On 1/26/21 11:14 AM, Vlastimil Babka wrote: > > > If it was a boot option, I would personally be for leaving hashing > > > enabled by > > > default, with opt-in boot

Re: [PATCH 0/2] introduce DUMP_PREFIX_UNHASHED for hex dumps

2021-01-26 Thread Timur Tabi
On 1/26/21 11:14 AM, Vlastimil Babka wrote: If it was a boot option, I would personally be for leaving hashing enabled by default, with opt-in boot option to disable it. A boot option would solve all my problems. I wouldn't need to recompile the kernel, and it would apply to all variations

Re: [PATCH 0/2] introduce DUMP_PREFIX_UNHASHED for hex dumps

2021-01-20 Thread Kees Cook
On Tue, Jan 19, 2021 at 12:18:17PM -0800, Randy Dunlap wrote: > On 1/19/21 11:45 AM, Kees Cook wrote: > > > > How about this so the base address is hashed once, with the offset added > > to it for each line instead of each line having a "new" hash that makes > > no sense: > > Yes, good patch.

Re: [PATCH 0/2] introduce DUMP_PREFIX_UNHASHED for hex dumps

2021-01-20 Thread Linus Torvalds
On Wed, Jan 20, 2021 at 1:19 AM Petr Mladek wrote: > > And we should definitely add Linus into CC when sending v2. > His expected opinion has been mentioned several times in this > thread. It would be better to avoid these speculations > and get his real opinion. IMHO, it is too late to add > him

Re: [PATCH 0/2] introduce DUMP_PREFIX_UNHASHED for hex dumps

2021-01-20 Thread Matthew Wilcox
On Wed, Jan 20, 2021 at 10:19:17AM +0100, Petr Mladek wrote: > And we should definitely add Linus into CC when sending v2. > His expected opinion has been mentioned several times in this > thread. It would be better to avoid these speculations > and get his real opinion. IMHO, it is too late to

Re: [PATCH 0/2] introduce DUMP_PREFIX_UNHASHED for hex dumps

2021-01-20 Thread Petr Mladek
On Tue 2021-01-19 13:55:29, Timur Tabi wrote: > On 1/19/21 1:45 PM, Kees Cook wrote: > > How about this so the base address is hashed once, with the offset added > > to it for each line instead of each line having a "new" hash that makes > > no sense: > > > > diff --git a/lib/hexdump.c

Re: [PATCH 0/2] introduce DUMP_PREFIX_UNHASHED for hex dumps

2021-01-19 Thread Timur Tabi
On 1/19/21 3:15 PM, Steven Rostedt wrote: When it's not related to any symbol, doesn't it still produce an offset with something close by, that could still give you information that's better than a hashed number. No. I often need the actual unhashed address in the hex dump so that I can see

Re: [PATCH 0/2] introduce DUMP_PREFIX_UNHASHED for hex dumps

2021-01-19 Thread Steven Rostedt
On Tue, 19 Jan 2021 14:49:17 -0600 Timur Tabi wrote: > On 1/19/21 2:10 PM, Steven Rostedt wrote: > > I'm curious, what is the result if you replaced %p with %pS? > > > > That way you get a kallsyms offset version of the output, which could still > > be very useful depending on what you are

Re: [PATCH 0/2] introduce DUMP_PREFIX_UNHASHED for hex dumps

2021-01-19 Thread Timur Tabi
On 1/19/21 2:10 PM, Steven Rostedt wrote: I'm curious, what is the result if you replaced %p with %pS? That way you get a kallsyms offset version of the output, which could still be very useful depending on what you are dumping. %pS versatile_init+0x0/0x110 The address is

Re: [PATCH 0/2] introduce DUMP_PREFIX_UNHASHED for hex dumps

2021-01-19 Thread Randy Dunlap
On 1/19/21 11:45 AM, Kees Cook wrote: > > How about this so the base address is hashed once, with the offset added > to it for each line instead of each line having a "new" hash that makes > no sense: Yes, good patch. Should have been like this to begin with IMO. > diff --git a/lib/hexdump.c

Re: [PATCH 0/2] introduce DUMP_PREFIX_UNHASHED for hex dumps

2021-01-19 Thread Steven Rostedt
On Tue, 19 Jan 2021 13:55:29 -0600 Timur Tabi wrote: > > case DUMP_PREFIX_ADDRESS: > > printk("%s%s%p: %s\n", > > - level, prefix_str, ptr + i, linebuf); > > + level, prefix_str, addr + i, linebuf); > > Well,

Re: [PATCH 0/2] introduce DUMP_PREFIX_UNHASHED for hex dumps

2021-01-19 Thread Kees Cook
On Tue, Jan 19, 2021 at 01:47:25AM +, Matthew Wilcox wrote: > On Tue, Jan 19, 2021 at 09:53:01AM +0900, Sergey Senozhatsky wrote: > > On (21/01/18 13:03), Timur Tabi wrote: > > > On 1/18/21 12:26 PM, Matthew Wilcox wrote: > > > > Don't make it easy. And don't make it look like they're doing >

Re: [PATCH 0/2] introduce DUMP_PREFIX_UNHASHED for hex dumps

2021-01-19 Thread Timur Tabi
On 1/19/21 1:45 PM, Kees Cook wrote: How about this so the base address is hashed once, with the offset added to it for each line instead of each line having a "new" hash that makes no sense: diff --git a/lib/hexdump.c b/lib/hexdump.c index 9301578f98e8..20264828752d 100644 --- a/lib/hexdump.c

Re: [PATCH 0/2] introduce DUMP_PREFIX_UNHASHED for hex dumps

2021-01-19 Thread Kees Cook
On Tue, Jan 19, 2021 at 07:38:24PM +0900, Sergey Senozhatsky wrote: > On (21/01/19 01:47), Matthew Wilcox wrote: > [..] > > > > > So maybe DUMP_PREFIX_UNHASHED can do the unhashed dump only when > > > CONFIG_DEBUG_KERNEL=y and fallback to DUMP_PREFIX_ADDRESS otherwise? > > > > Distros enable

Re: [PATCH 0/2] introduce DUMP_PREFIX_UNHASHED for hex dumps

2021-01-19 Thread Sergey Senozhatsky
On (21/01/19 01:47), Matthew Wilcox wrote: [..] > > > So maybe DUMP_PREFIX_UNHASHED can do the unhashed dump only when > > CONFIG_DEBUG_KERNEL=y and fallback to DUMP_PREFIX_ADDRESS otherwise? > > Distros enable CONFIG_DEBUG_KERNEL. Oh, I see. > If you want to add CONFIG_DEBUG_LEAK_ADDRESSES,

Re: [PATCH 0/2] introduce DUMP_PREFIX_UNHASHED for hex dumps

2021-01-18 Thread Timur Tabi
On 1/18/21 6:53 PM, Sergey Senozhatsky wrote: I like the idea of a more radical name, e.g. DUMP_PREFIX_RAW_POINTERS or something similar. Is "raw pointer" the common term for unhashed pointers?

Re: [PATCH 0/2] introduce DUMP_PREFIX_UNHASHED for hex dumps

2021-01-18 Thread Matthew Wilcox
On Tue, Jan 19, 2021 at 09:53:01AM +0900, Sergey Senozhatsky wrote: > On (21/01/18 13:03), Timur Tabi wrote: > > On 1/18/21 12:26 PM, Matthew Wilcox wrote: > > > Don't make it easy. And don't make it look like they're doing > > > something innocent. DUMP_PREFIX_SECURITY_HOLE would be OK > > > by

Re: [PATCH 0/2] introduce DUMP_PREFIX_UNHASHED for hex dumps

2021-01-18 Thread Sergey Senozhatsky
On (21/01/18 13:03), Timur Tabi wrote: > On 1/18/21 12:26 PM, Matthew Wilcox wrote: > > Don't make it easy. And don't make it look like they're doing > > something innocent. DUMP_PREFIX_SECURITY_HOLE would be OK > > by me. DUMP_PREFIX_LEAK_INFORMATION would work fine too. > >

Re: [PATCH 0/2] introduce DUMP_PREFIX_UNHASHED for hex dumps

2021-01-18 Thread Timur Tabi
On 1/18/21 12:26 PM, Matthew Wilcox wrote: Don't make it easy. And don't make it look like they're doing something innocent. DUMP_PREFIX_SECURITY_HOLE would be OK by me. DUMP_PREFIX_LEAK_INFORMATION would work fine too. DUMP_PREFIX_MAKE_ATTACKERS_LIFE_EASY might be a bit too far. It's

Re: [PATCH 0/2] introduce DUMP_PREFIX_UNHASHED for hex dumps

2021-01-18 Thread Matthew Wilcox
On Sat, Jan 16, 2021 at 04:09:48PM -0600, Timur Tabi wrote: > First patch updates print_hex_dump() and related functions to > allow callers to print hex dumps with unhashed addresses. It > adds a new prefix type, so existing code is unchanged. > > Second patch changes a page poising function to

[PATCH 0/2] introduce DUMP_PREFIX_UNHASHED for hex dumps

2021-01-16 Thread Timur Tabi
First patch updates print_hex_dump() and related functions to allow callers to print hex dumps with unhashed addresses. It adds a new prefix type, so existing code is unchanged. Second patch changes a page poising function to use the new address type. This is just an example of a change. If