Re: [PATCH V8 0/2] printk: hash addresses printed with %p

[Vinod Koul]

On Tue, Oct 31, 2017 at 04:33:13PM -0700, Joe Perches wrote:
> On Wed, 2017-11-01 at 10:16 +1100, Tobin C. Harding wrote:

> > Cool, thanks Joe I'll keep this in mind for when we get to %pa.
> 
> fyi:  There are more of these misuses of 0x%pa now:
> 
> $ git grep -E -n "0[xX]%pa[dp]?\b"
> drivers/dma/at_hdmac_regs.h:388: "  desc: s%pad d%pad 
> ctrl0x%x:0x%x l0x%pad\n",
> drivers/dma/coh901318.c:1322:   dev_vdbg(COHC_2_DEV(cohc), "i %d, lli 
> %p, ctrl 0x%x, src 0x%pad"
> drivers/dma/coh901318.c:1323:", dst 0x%pad, link 0x%pad 
> virt_link_addr 0x%p\n",
> drivers/dma/coh901318.c:2234:"[%s] channel %d src 0x%pad dest 
> 0x%pad size %zu\n",

Fixed and sent patches for these two.

-- 
~Vinod

Re: [PATCH V8 0/2] printk: hash addresses printed with %p

[Vinod Koul]

On Tue, Oct 31, 2017 at 04:33:13PM -0700, Joe Perches wrote:
> On Wed, 2017-11-01 at 10:16 +1100, Tobin C. Harding wrote:

> > Cool, thanks Joe I'll keep this in mind for when we get to %pa.
> 
> fyi:  There are more of these misuses of 0x%pa now:
> 
> $ git grep -E -n "0[xX]%pa[dp]?\b"
> drivers/dma/at_hdmac_regs.h:388: "  desc: s%pad d%pad 
> ctrl0x%x:0x%x l0x%pad\n",
> drivers/dma/coh901318.c:1322:   dev_vdbg(COHC_2_DEV(cohc), "i %d, lli 
> %p, ctrl 0x%x, src 0x%pad"
> drivers/dma/coh901318.c:1323:", dst 0x%pad, link 0x%pad 
> virt_link_addr 0x%p\n",
> drivers/dma/coh901318.c:2234:"[%s] channel %d src 0x%pad dest 
> 0x%pad size %zu\n",

Fixed and sent patches for these two.

-- 
~Vinod

Re: [PATCH V8 0/2] printk: hash addresses printed with %p

[Sergey Senozhatsky]

On (11/02/17 21:14), Tobin C. Harding wrote:
[..]
> I can put my email address if there is not a better option.

sounds good.


> > hm... just a huge pile of if's
> > 
> > if (is_vmalloc_addr(addr))
> > do_hashing(addr);
> > else if (__module_address(addr))
> > do_hashing(addr);
> > else if (is_kernel(addr) || is_kernel_inittext(addr))
> > ...
> > 
> > but that's going to be really messy and "iffy".
> 
> This is the only suggestion we have so far.
> 

well... one more: check if we can safely dereference it. if so
it's a pointer, probably :)

if (!probe_kernel_address(addr, p))
do_hashing(addr);


just an idea.

-ss

Re: [PATCH V8 0/2] printk: hash addresses printed with %p

[Sergey Senozhatsky]

On (11/02/17 21:14), Tobin C. Harding wrote:
[..]
> I can put my email address if there is not a better option.

sounds good.


> > hm... just a huge pile of if's
> > 
> > if (is_vmalloc_addr(addr))
> > do_hashing(addr);
> > else if (__module_address(addr))
> > do_hashing(addr);
> > else if (is_kernel(addr) || is_kernel_inittext(addr))
> > ...
> > 
> > but that's going to be really messy and "iffy".
> 
> This is the only suggestion we have so far.
> 

well... one more: check if we can safely dereference it. if so
it's a pointer, probably :)

if (!probe_kernel_address(addr, p))
do_hashing(addr);


just an idea.

-ss

RE: [PATCH V8 0/2] printk: hash addresses printed with %p

[Roberts, William C]

> -Original Message-
> From: Tobin C. Harding [mailto:m...@tobin.cc]
> Sent: Thursday, November 2, 2017 3:15 AM
> To: Sergey Senozhatsky <sergey.senozhatsky.w...@gmail.com>
> Cc: Sergey Senozhatsky <sergey.senozhat...@gmail.com>; kernel-
> harden...@lists.openwall.com; Jason A. Donenfeld <ja...@zx2c4.com>;
> Theodore Ts'o <ty...@mit.edu>; Linus Torvalds <torvalds@linux-
> foundation.org>; Kees Cook <keesc...@chromium.org>; Paolo Bonzini
> <pbonz...@redhat.com>; Tycho Andersen <ty...@docker.com>; Roberts,
> William C <william.c.robe...@intel.com>; Tejun Heo <t...@kernel.org>; Jordan
> Glover <golden_mille...@protonmail.ch>; Greg KH
> <gre...@linuxfoundation.org>; Petr Mladek <pmla...@suse.com>; Joe
> Perches <j...@perches.com>; Ian Campbell <i...@hellion.org.uk>; Catalin 
> Marinas
> <catalin.mari...@arm.com>; Will Deacon <wilal.dea...@arm.com>; Steven
> Rostedt <rost...@goodmis.org>; Chris Fries <cfr...@google.com>; Dave
> Weinstein <olo...@google.com>; Daniel Micay <danielmi...@gmail.com>; Djalal
> Harouni <tix...@gmail.com>; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH V8 0/2] printk: hash addresses printed with %p
> 
> On Thu, Nov 02, 2017 at 05:23:44PM +0900, Sergey Senozhatsky wrote:
> > On (11/01/17 10:35), Tobin C. Harding wrote:
> > [..]
> > > Yes. The question has been raised will we be here again in 6 years
> > > time trying to fix all the uses of %x. And there are already 29K
> > > uses of %[xX] in tree, which of these are leaking addresses? This is why 
> > > Linus'
> > > has commented that really effort should be directed at finding the
> > > leaks as they happen (in procfs, sysfs, dmesg) instead of fixing
> > > this in the code.
> >
> > got it. thanks.
> >
> > > So far I haven't been able to come up with any meaningful way to do
> > > this on 32 bit machines. There is a patch adding a script to catch
> > > leaks on 64 bit machines in flight.
> >
> > who is expected to run that script?
> 
> If one person runs it and finds one leaking address, I'd say it wast worth 
> writing. If
> a bunch of people with different set ups run it and we find a bunch of leaking
> addresses, WIN!

I wonder if the 0 day testing robot could run it

> 
> Your comment did give me the idea of adding some output to the command
> offering an email address to send suspicious output for those who do not wish 
> to
> investigate it further. I can put my email address if there is not a better 
> option.
> 
> > BTW, can BPF/eBPF printk addresses?
> 
> I know absolutely zero about BPF/eBPF. I guess now is a good time to learn.
> 
> > > This patch needs to be a small part of a continued effort to stop
> > > the leaks if we want to have any hope of stopping them.
> > >
> > > If you have any suggestions on dealing with %x please do say. We
> > > have code changes, compiler warnings, and checkpatch - none of which
> > > immediately seem great.
> >
> > hm... just a huge pile of if's
> >
> > if (is_vmalloc_addr(addr))
> > do_hashing(addr);
> > else if (__module_address(addr))
> > do_hashing(addr);
> > else if (is_kernel(addr) || is_kernel_inittext(addr))
> > ...
> >
> > but that's going to be really messy and "iffy".
> 
> This is the only suggestion we have so far.
> 
> thanks,
> Tobin.

RE: [PATCH V8 0/2] printk: hash addresses printed with %p

[Roberts, William C]

> -Original Message-
> From: Tobin C. Harding [mailto:m...@tobin.cc]
> Sent: Thursday, November 2, 2017 3:15 AM
> To: Sergey Senozhatsky 
> Cc: Sergey Senozhatsky ; kernel-
> harden...@lists.openwall.com; Jason A. Donenfeld ;
> Theodore Ts'o ; Linus Torvalds  foundation.org>; Kees Cook ; Paolo Bonzini
> ; Tycho Andersen ; Roberts,
> William C ; Tejun Heo ; Jordan
> Glover ; Greg KH
> ; Petr Mladek ; Joe
> Perches ; Ian Campbell ; Catalin 
> Marinas
> ; Will Deacon ; Steven
> Rostedt ; Chris Fries ; Dave
> Weinstein ; Daniel Micay ; Djalal
> Harouni ; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH V8 0/2] printk: hash addresses printed with %p
> 
> On Thu, Nov 02, 2017 at 05:23:44PM +0900, Sergey Senozhatsky wrote:
> > On (11/01/17 10:35), Tobin C. Harding wrote:
> > [..]
> > > Yes. The question has been raised will we be here again in 6 years
> > > time trying to fix all the uses of %x. And there are already 29K
> > > uses of %[xX] in tree, which of these are leaking addresses? This is why 
> > > Linus'
> > > has commented that really effort should be directed at finding the
> > > leaks as they happen (in procfs, sysfs, dmesg) instead of fixing
> > > this in the code.
> >
> > got it. thanks.
> >
> > > So far I haven't been able to come up with any meaningful way to do
> > > this on 32 bit machines. There is a patch adding a script to catch
> > > leaks on 64 bit machines in flight.
> >
> > who is expected to run that script?
> 
> If one person runs it and finds one leaking address, I'd say it wast worth 
> writing. If
> a bunch of people with different set ups run it and we find a bunch of leaking
> addresses, WIN!

I wonder if the 0 day testing robot could run it

> 
> Your comment did give me the idea of adding some output to the command
> offering an email address to send suspicious output for those who do not wish 
> to
> investigate it further. I can put my email address if there is not a better 
> option.
> 
> > BTW, can BPF/eBPF printk addresses?
> 
> I know absolutely zero about BPF/eBPF. I guess now is a good time to learn.
> 
> > > This patch needs to be a small part of a continued effort to stop
> > > the leaks if we want to have any hope of stopping them.
> > >
> > > If you have any suggestions on dealing with %x please do say. We
> > > have code changes, compiler warnings, and checkpatch - none of which
> > > immediately seem great.
> >
> > hm... just a huge pile of if's
> >
> > if (is_vmalloc_addr(addr))
> > do_hashing(addr);
> > else if (__module_address(addr))
> > do_hashing(addr);
> > else if (is_kernel(addr) || is_kernel_inittext(addr))
> > ...
> >
> > but that's going to be really messy and "iffy".
> 
> This is the only suggestion we have so far.
> 
> thanks,
> Tobin.

Re: [PATCH V8 0/2] printk: hash addresses printed with %p

[Tobin C. Harding]

On Thu, Nov 02, 2017 at 05:23:44PM +0900, Sergey Senozhatsky wrote:
> On (11/01/17 10:35), Tobin C. Harding wrote:
> [..]
> > Yes. The question has been raised will we be here again in 6 years time
> > trying to fix all the uses of %x. And there are already 29K uses of
> > %[xX] in tree, which of these are leaking addresses? This is why Linus'
> > has commented that really effort should be directed at finding the leaks
> > as they happen (in procfs, sysfs, dmesg) instead of fixing this in
> > the code.
> 
> got it. thanks.
> 
> > So far I haven't been able to come up with any meaningful way
> > to do this on 32 bit machines. There is a patch adding a script to catch
> > leaks on 64 bit machines in flight.
> 
> who is expected to run that script?

If one person runs it and finds one leaking address, I'd say it wast
worth writing. If a bunch of people with different set ups run it and we
find a bunch of leaking addresses, WIN!

Your comment did give me the idea of adding some output to the command
offering an email address to send suspicious output for those who do not
wish to investigate it further. I can put my email address if there is
not a better option. 

> BTW, can BPF/eBPF printk addresses?

I know absolutely zero about BPF/eBPF. I guess now is a good time to learn.

> > This patch needs to be a small part of a continued effort to stop the
> > leaks if we want to have any hope of stopping them.
> > 
> > If you have any suggestions on dealing with %x please do say. We have
> > code changes, compiler warnings, and checkpatch - none of which
> > immediately seem great.
> 
> hm... just a huge pile of if's
> 
>   if (is_vmalloc_addr(addr))
>   do_hashing(addr);
>   else if (__module_address(addr))
>   do_hashing(addr);
>   else if (is_kernel(addr) || is_kernel_inittext(addr))
>   ...
> 
> but that's going to be really messy and "iffy".

This is the only suggestion we have so far.

thanks,
Tobin.

Re: [PATCH V8 0/2] printk: hash addresses printed with %p

[Tobin C. Harding]

On Thu, Nov 02, 2017 at 05:23:44PM +0900, Sergey Senozhatsky wrote:
> On (11/01/17 10:35), Tobin C. Harding wrote:
> [..]
> > Yes. The question has been raised will we be here again in 6 years time
> > trying to fix all the uses of %x. And there are already 29K uses of
> > %[xX] in tree, which of these are leaking addresses? This is why Linus'
> > has commented that really effort should be directed at finding the leaks
> > as they happen (in procfs, sysfs, dmesg) instead of fixing this in
> > the code.
> 
> got it. thanks.
> 
> > So far I haven't been able to come up with any meaningful way
> > to do this on 32 bit machines. There is a patch adding a script to catch
> > leaks on 64 bit machines in flight.
> 
> who is expected to run that script?

If one person runs it and finds one leaking address, I'd say it wast
worth writing. If a bunch of people with different set ups run it and we
find a bunch of leaking addresses, WIN!

Your comment did give me the idea of adding some output to the command
offering an email address to send suspicious output for those who do not
wish to investigate it further. I can put my email address if there is
not a better option. 

> BTW, can BPF/eBPF printk addresses?

I know absolutely zero about BPF/eBPF. I guess now is a good time to learn.

> > This patch needs to be a small part of a continued effort to stop the
> > leaks if we want to have any hope of stopping them.
> > 
> > If you have any suggestions on dealing with %x please do say. We have
> > code changes, compiler warnings, and checkpatch - none of which
> > immediately seem great.
> 
> hm... just a huge pile of if's
> 
>   if (is_vmalloc_addr(addr))
>   do_hashing(addr);
>   else if (__module_address(addr))
>   do_hashing(addr);
>   else if (is_kernel(addr) || is_kernel_inittext(addr))
>   ...
> 
> but that's going to be really messy and "iffy".

This is the only suggestion we have so far.

thanks,
Tobin.

Re: [PATCH V8 0/2] printk: hash addresses printed with %p

[Sergey Senozhatsky]

On (11/01/17 10:35), Tobin C. Harding wrote:
[..]
> Yes. The question has been raised will we be here again in 6 years time
> trying to fix all the uses of %x. And there are already 29K uses of
> %[xX] in tree, which of these are leaking addresses? This is why Linus'
> has commented that really effort should be directed at finding the leaks
> as they happen (in procfs, sysfs, dmesg) instead of fixing this in
> the code.

got it. thanks.

> So far I haven't been able to come up with any meaningful way
> to do this on 32 bit machines. There is a patch adding a script to catch
> leaks on 64 bit machines in flight.

who is expected to run that script?

BTW, can BPF/eBPF printk addresses?

> This patch needs to be a small part of a continued effort to stop the
> leaks if we want to have any hope of stopping them.
> 
> If you have any suggestions on dealing with %x please do say. We have
> code changes, compiler warnings, and checkpatch - none of which
> immediately seem great.

hm... just a huge pile of if's

if (is_vmalloc_addr(addr))
do_hashing(addr);
else if (__module_address(addr))
do_hashing(addr);
else if (is_kernel(addr) || is_kernel_inittext(addr))
...

but that's going to be really messy and "iffy".

-ss

Re: [PATCH V8 0/2] printk: hash addresses printed with %p

[Sergey Senozhatsky]

On (11/01/17 10:35), Tobin C. Harding wrote:
[..]
> Yes. The question has been raised will we be here again in 6 years time
> trying to fix all the uses of %x. And there are already 29K uses of
> %[xX] in tree, which of these are leaking addresses? This is why Linus'
> has commented that really effort should be directed at finding the leaks
> as they happen (in procfs, sysfs, dmesg) instead of fixing this in
> the code.

got it. thanks.

> So far I haven't been able to come up with any meaningful way
> to do this on 32 bit machines. There is a patch adding a script to catch
> leaks on 64 bit machines in flight.

who is expected to run that script?

BTW, can BPF/eBPF printk addresses?

> This patch needs to be a small part of a continued effort to stop the
> leaks if we want to have any hope of stopping them.
> 
> If you have any suggestions on dealing with %x please do say. We have
> code changes, compiler warnings, and checkpatch - none of which
> immediately seem great.

hm... just a huge pile of if's

if (is_vmalloc_addr(addr))
do_hashing(addr);
else if (__module_address(addr))
do_hashing(addr);
else if (is_kernel(addr) || is_kernel_inittext(addr))
...

but that's going to be really messy and "iffy".

-ss

Re: [PATCH V8 0/2] printk: hash addresses printed with %p

[Tobin C. Harding]

On Fri, Oct 27, 2017 at 10:33:01PM +0900, Sergey Senozhatsky wrote:
> On (10/26/17 13:53), Tobin C. Harding wrote:
> > Currently there are many places in the kernel where addresses are being
> > printed using an unadorned %p. Kernel pointers should be printed using
> > %pK allowing some control via the kptr_restrict sysctl. Exposing
> > addresses gives attackers sensitive information about the kernel layout
> > in memory.
> > 
> > We can reduce the attack surface by hashing all addresses printed with
> > %p. This will of course break some users, forcing code printing needed
> > addresses to be updated.
> > 
> > With this version we include hashing of malformed specifiers also.
> > Malformed specifiers include incomplete (e.g %pi) and also non-existent
> > specifiers. checkpatch should warn for non-existent specifiers but
> > AFAICT won't warn for incomplete specifiers.
> > 
> > Here is the behaviour that this set implements.
> > 
> > For kpt_restrict==0
> > 
> > Randomness not ready:
> >   printed with %p:  (pointer)  # NOTE: with padding
> > Valid pointer:
> >   printed with %pK: deadbeefdeadbeef
> >   printed with %p:  0xdeadbeef
> >   malformed specifier (eg %i):  0xdeadbeef
> > NULL pointer:
> >   printed with %pK: 
> >   printed with %p:  (null)   # NOTE: no padding
> >   malformed specifier (eg %i):  (null)
> 
> a quick question:
>  do we care about cases when kernel pointers are printed with %x/%X and
>  not with %p?

Yes. The question has been raised will we be here again in 6 years time
trying to fix all the uses of %x. And there are already 29K uses of
%[xX] in tree, which of these are leaking addresses? This is why Linus'
has commented that really effort should be directed at finding the leaks
as they happen (in procfs, sysfs, dmesg) instead of fixing this in
the code. So far I haven't been able to come up with any meaningful way
to do this on 32 bit machines. There is a patch adding a script to catch
leaks on 64 bit machines in flight.

This patch needs to be a small part of a continued effort to stop the
leaks if we want to have any hope of stopping them.

If you have any suggestions on dealing with %x please do say. We have
code changes, compiler warnings, and checkpatch - none of which
immediately seem great.

thanks,
Tobin.

Re: [PATCH V8 0/2] printk: hash addresses printed with %p

[Tobin C. Harding]

On Fri, Oct 27, 2017 at 10:33:01PM +0900, Sergey Senozhatsky wrote:
> On (10/26/17 13:53), Tobin C. Harding wrote:
> > Currently there are many places in the kernel where addresses are being
> > printed using an unadorned %p. Kernel pointers should be printed using
> > %pK allowing some control via the kptr_restrict sysctl. Exposing
> > addresses gives attackers sensitive information about the kernel layout
> > in memory.
> > 
> > We can reduce the attack surface by hashing all addresses printed with
> > %p. This will of course break some users, forcing code printing needed
> > addresses to be updated.
> > 
> > With this version we include hashing of malformed specifiers also.
> > Malformed specifiers include incomplete (e.g %pi) and also non-existent
> > specifiers. checkpatch should warn for non-existent specifiers but
> > AFAICT won't warn for incomplete specifiers.
> > 
> > Here is the behaviour that this set implements.
> > 
> > For kpt_restrict==0
> > 
> > Randomness not ready:
> >   printed with %p:  (pointer)  # NOTE: with padding
> > Valid pointer:
> >   printed with %pK: deadbeefdeadbeef
> >   printed with %p:  0xdeadbeef
> >   malformed specifier (eg %i):  0xdeadbeef
> > NULL pointer:
> >   printed with %pK: 
> >   printed with %p:  (null)   # NOTE: no padding
> >   malformed specifier (eg %i):  (null)
> 
> a quick question:
>  do we care about cases when kernel pointers are printed with %x/%X and
>  not with %p?

Yes. The question has been raised will we be here again in 6 years time
trying to fix all the uses of %x. And there are already 29K uses of
%[xX] in tree, which of these are leaking addresses? This is why Linus'
has commented that really effort should be directed at finding the leaks
as they happen (in procfs, sysfs, dmesg) instead of fixing this in
the code. So far I haven't been able to come up with any meaningful way
to do this on 32 bit machines. There is a patch adding a script to catch
leaks on 64 bit machines in flight.

This patch needs to be a small part of a continued effort to stop the
leaks if we want to have any hope of stopping them.

If you have any suggestions on dealing with %x please do say. We have
code changes, compiler warnings, and checkpatch - none of which
immediately seem great.

thanks,
Tobin.

Re: [PATCH V8 0/2] printk: hash addresses printed with %p

[Joe Perches]

On Wed, 2017-11-01 at 10:16 +1100, Tobin C. Harding wrote:
> On Mon, Oct 30, 2017 at 07:08:48PM -0700, Joe Perches wrote:
> > On Tue, 2017-10-31 at 09:33 +1100, Tobin C. Harding wrote:
> > > On Mon, Oct 30, 2017 at 03:03:21PM -0700, Kees Cook wrote:
> > > > On Wed, Oct 25, 2017 at 7:53 PM, Tobin C. Harding  wrote:
> > > > > Here is the behaviour that this set implements.
> > > > > 
> > > > > For kpt_restrict==0
> > > > > 
> > > > > Randomness not ready:
> > > > >   printed with %p:  (pointer)  # NOTE: with 
> > > > > padding
> > > > > Valid pointer:
> > > > >   printed with %pK: deadbeefdeadbeef
> > > > >   printed with %p:  0xdeadbeef
> > > > >   malformed specifier (eg %i):  0xdeadbeef
> > > > 
> > > > I really think we can't include SPECIAL unless _every_ callsite of %p
> > > > is actually doing "0x%p", and then we're replacing all of those. We're
> > > > not doing that, though...
> > > > 
> > > > $ git grep '%p\b' | wc -l
> > > > 12766
> > > > $ git grep '0x%p\b' | wc -l
> > > > 18370x
> > > > 
> > > > If we need some kind of special marking that this is a hashed
> > > > variable, that should be something other than "0x". If we're using the
> > > > existing "(null)" and new "(pointer)" text, maybe "(hash:xx)"
> > > > should be used instead? Then the (rare) callers with 0x become
> > > > "0x(hash:)" and naked callers produce "(hash:)".
> > > > 
> > > > I think the first step for this is to just leave SPECIAL out.
> > > 
> > > Thanks Kees. V9 leaves SPECIAL out. Also V9 prints the whole 64 bit
> > > address with the first 32 bits masked to zero. The intent being to _not_
> > > change the output format from what it currently is. So it will look like
> > > this; 
> > > 
> > >   c09e81d0
> > > 
> > > What do you think?
> > > 
> > > Amusingly I think this whole conversation is going to come up again
> > > when we do %pa, in inverse, since %pa currently does us SPECIAL.
> > 
> > I once sent a patch set to remove SPECIAL from %pa
> > and add 0x where necessary.
> > 
> > https://patchwork.kernel.org/patch/3875471/
> > 
> > After that didn't happen, I removed the duplicated
> > 0x%pa with a sed.
> > 
> > https://patchwork.kernel.org/patch/8509421/
> > 
> > Sending a treewide sed patch would be fine with me.
> 
> Cool, thanks Joe I'll keep this in mind for when we get to %pa.

fyi:  There are more of these misuses of 0x%pa now:

$ git grep -E -n "0[xX]%pa[dp]?\b"
drivers/dma/at_hdmac_regs.h:388: "  desc: s%pad d%pad 
ctrl0x%x:0x%x l0x%pad\n",
drivers/dma/coh901318.c:1322:   dev_vdbg(COHC_2_DEV(cohc), "i %d, lli 
%p, ctrl 0x%x, src 0x%pad"
drivers/dma/coh901318.c:1323:", dst 0x%pad, link 0x%pad 
virt_link_addr 0x%p\n",
drivers/dma/coh901318.c:2234:"[%s] channel %d src 0x%pad dest 
0x%pad size %zu\n",
drivers/media/platform/sti/delta/delta-mem.c:35:"%s allocate %d 
bytes of HW memory @(virt=0x%p, phy=0x%pad): %s\n",
drivers/media/platform/sti/delta/delta-mem.c:46:"%s free %d 
bytes of HW memory @(virt=0x%p, phy=0x%pad): %s\n",
drivers/media/platform/sti/delta/delta-v4l2.c:1147: 
dev_dbg(delta->dev, "%s au[%d] prepared; virt=0x%p, phy=0x%pad\n",
drivers/media/platform/sti/delta/delta-v4l2.c:1503: "%s 
frame[%d] prepared; virt=0x%p, phy=0x%pad\n",
drivers/media/platform/stm32/stm32-dcmi.c:486:  dev_dbg(dcmi->dev, 
"buffer[%d] phy=0x%pad size=%zu\n",
drivers/media/platform/ti-vpe/cal.c:496:cal_info(dev, "CAL Registers @ 
0x%pa:\n", >res->start);

Re: [PATCH V8 0/2] printk: hash addresses printed with %p

[Joe Perches]

On Wed, 2017-11-01 at 10:16 +1100, Tobin C. Harding wrote:
> On Mon, Oct 30, 2017 at 07:08:48PM -0700, Joe Perches wrote:
> > On Tue, 2017-10-31 at 09:33 +1100, Tobin C. Harding wrote:
> > > On Mon, Oct 30, 2017 at 03:03:21PM -0700, Kees Cook wrote:
> > > > On Wed, Oct 25, 2017 at 7:53 PM, Tobin C. Harding  wrote:
> > > > > Here is the behaviour that this set implements.
> > > > > 
> > > > > For kpt_restrict==0
> > > > > 
> > > > > Randomness not ready:
> > > > >   printed with %p:  (pointer)  # NOTE: with 
> > > > > padding
> > > > > Valid pointer:
> > > > >   printed with %pK: deadbeefdeadbeef
> > > > >   printed with %p:  0xdeadbeef
> > > > >   malformed specifier (eg %i):  0xdeadbeef
> > > > 
> > > > I really think we can't include SPECIAL unless _every_ callsite of %p
> > > > is actually doing "0x%p", and then we're replacing all of those. We're
> > > > not doing that, though...
> > > > 
> > > > $ git grep '%p\b' | wc -l
> > > > 12766
> > > > $ git grep '0x%p\b' | wc -l
> > > > 18370x
> > > > 
> > > > If we need some kind of special marking that this is a hashed
> > > > variable, that should be something other than "0x". If we're using the
> > > > existing "(null)" and new "(pointer)" text, maybe "(hash:xx)"
> > > > should be used instead? Then the (rare) callers with 0x become
> > > > "0x(hash:)" and naked callers produce "(hash:)".
> > > > 
> > > > I think the first step for this is to just leave SPECIAL out.
> > > 
> > > Thanks Kees. V9 leaves SPECIAL out. Also V9 prints the whole 64 bit
> > > address with the first 32 bits masked to zero. The intent being to _not_
> > > change the output format from what it currently is. So it will look like
> > > this; 
> > > 
> > >   c09e81d0
> > > 
> > > What do you think?
> > > 
> > > Amusingly I think this whole conversation is going to come up again
> > > when we do %pa, in inverse, since %pa currently does us SPECIAL.
> > 
> > I once sent a patch set to remove SPECIAL from %pa
> > and add 0x where necessary.
> > 
> > https://patchwork.kernel.org/patch/3875471/
> > 
> > After that didn't happen, I removed the duplicated
> > 0x%pa with a sed.
> > 
> > https://patchwork.kernel.org/patch/8509421/
> > 
> > Sending a treewide sed patch would be fine with me.
> 
> Cool, thanks Joe I'll keep this in mind for when we get to %pa.

fyi:  There are more of these misuses of 0x%pa now:

$ git grep -E -n "0[xX]%pa[dp]?\b"
drivers/dma/at_hdmac_regs.h:388: "  desc: s%pad d%pad 
ctrl0x%x:0x%x l0x%pad\n",
drivers/dma/coh901318.c:1322:   dev_vdbg(COHC_2_DEV(cohc), "i %d, lli 
%p, ctrl 0x%x, src 0x%pad"
drivers/dma/coh901318.c:1323:", dst 0x%pad, link 0x%pad 
virt_link_addr 0x%p\n",
drivers/dma/coh901318.c:2234:"[%s] channel %d src 0x%pad dest 
0x%pad size %zu\n",
drivers/media/platform/sti/delta/delta-mem.c:35:"%s allocate %d 
bytes of HW memory @(virt=0x%p, phy=0x%pad): %s\n",
drivers/media/platform/sti/delta/delta-mem.c:46:"%s free %d 
bytes of HW memory @(virt=0x%p, phy=0x%pad): %s\n",
drivers/media/platform/sti/delta/delta-v4l2.c:1147: 
dev_dbg(delta->dev, "%s au[%d] prepared; virt=0x%p, phy=0x%pad\n",
drivers/media/platform/sti/delta/delta-v4l2.c:1503: "%s 
frame[%d] prepared; virt=0x%p, phy=0x%pad\n",
drivers/media/platform/stm32/stm32-dcmi.c:486:  dev_dbg(dcmi->dev, 
"buffer[%d] phy=0x%pad size=%zu\n",
drivers/media/platform/ti-vpe/cal.c:496:cal_info(dev, "CAL Registers @ 
0x%pa:\n", >res->start);

Re: [PATCH V8 0/2] printk: hash addresses printed with %p

[Tobin C. Harding]

On Mon, Oct 30, 2017 at 07:08:48PM -0700, Joe Perches wrote:
> On Tue, 2017-10-31 at 09:33 +1100, Tobin C. Harding wrote:
> > On Mon, Oct 30, 2017 at 03:03:21PM -0700, Kees Cook wrote:
> > > On Wed, Oct 25, 2017 at 7:53 PM, Tobin C. Harding  wrote:
> > > > Here is the behaviour that this set implements.
> > > > 
> > > > For kpt_restrict==0
> > > > 
> > > > Randomness not ready:
> > > >   printed with %p:  (pointer)  # NOTE: with padding
> > > > Valid pointer:
> > > >   printed with %pK: deadbeefdeadbeef
> > > >   printed with %p:  0xdeadbeef
> > > >   malformed specifier (eg %i):  0xdeadbeef
> > > 
> > > I really think we can't include SPECIAL unless _every_ callsite of %p
> > > is actually doing "0x%p", and then we're replacing all of those. We're
> > > not doing that, though...
> > > 
> > > $ git grep '%p\b' | wc -l
> > > 12766
> > > $ git grep '0x%p\b' | wc -l
> > > 18370x
> > > 
> > > If we need some kind of special marking that this is a hashed
> > > variable, that should be something other than "0x". If we're using the
> > > existing "(null)" and new "(pointer)" text, maybe "(hash:xx)"
> > > should be used instead? Then the (rare) callers with 0x become
> > > "0x(hash:)" and naked callers produce "(hash:)".
> > > 
> > > I think the first step for this is to just leave SPECIAL out.
> > 
> > Thanks Kees. V9 leaves SPECIAL out. Also V9 prints the whole 64 bit
> > address with the first 32 bits masked to zero. The intent being to _not_
> > change the output format from what it currently is. So it will look like
> > this; 
> > 
> > c09e81d0
> > 
> > What do you think?
> > 
> > Amusingly I think this whole conversation is going to come up again
> > when we do %pa, in inverse, since %pa currently does us SPECIAL.
> 
> I once sent a patch set to remove SPECIAL from %pa
> and add 0x where necessary.
> 
> https://patchwork.kernel.org/patch/3875471/
> 
> After that didn't happen, I removed the duplicated
> 0x%pa with a sed.
> 
> https://patchwork.kernel.org/patch/8509421/
> 
> Sending a treewide sed patch would be fine with me.

Cool, thanks Joe I'll keep this in mind for when we get to %pa.

thanks,
Tobin.

Re: [PATCH V8 0/2] printk: hash addresses printed with %p

[Tobin C. Harding]

On Mon, Oct 30, 2017 at 07:08:48PM -0700, Joe Perches wrote:
> On Tue, 2017-10-31 at 09:33 +1100, Tobin C. Harding wrote:
> > On Mon, Oct 30, 2017 at 03:03:21PM -0700, Kees Cook wrote:
> > > On Wed, Oct 25, 2017 at 7:53 PM, Tobin C. Harding  wrote:
> > > > Here is the behaviour that this set implements.
> > > > 
> > > > For kpt_restrict==0
> > > > 
> > > > Randomness not ready:
> > > >   printed with %p:  (pointer)  # NOTE: with padding
> > > > Valid pointer:
> > > >   printed with %pK: deadbeefdeadbeef
> > > >   printed with %p:  0xdeadbeef
> > > >   malformed specifier (eg %i):  0xdeadbeef
> > > 
> > > I really think we can't include SPECIAL unless _every_ callsite of %p
> > > is actually doing "0x%p", and then we're replacing all of those. We're
> > > not doing that, though...
> > > 
> > > $ git grep '%p\b' | wc -l
> > > 12766
> > > $ git grep '0x%p\b' | wc -l
> > > 18370x
> > > 
> > > If we need some kind of special marking that this is a hashed
> > > variable, that should be something other than "0x". If we're using the
> > > existing "(null)" and new "(pointer)" text, maybe "(hash:xx)"
> > > should be used instead? Then the (rare) callers with 0x become
> > > "0x(hash:)" and naked callers produce "(hash:)".
> > > 
> > > I think the first step for this is to just leave SPECIAL out.
> > 
> > Thanks Kees. V9 leaves SPECIAL out. Also V9 prints the whole 64 bit
> > address with the first 32 bits masked to zero. The intent being to _not_
> > change the output format from what it currently is. So it will look like
> > this; 
> > 
> > c09e81d0
> > 
> > What do you think?
> > 
> > Amusingly I think this whole conversation is going to come up again
> > when we do %pa, in inverse, since %pa currently does us SPECIAL.
> 
> I once sent a patch set to remove SPECIAL from %pa
> and add 0x where necessary.
> 
> https://patchwork.kernel.org/patch/3875471/
> 
> After that didn't happen, I removed the duplicated
> 0x%pa with a sed.
> 
> https://patchwork.kernel.org/patch/8509421/
> 
> Sending a treewide sed patch would be fine with me.

Cool, thanks Joe I'll keep this in mind for when we get to %pa.

thanks,
Tobin.

Re: [PATCH V8 0/2] printk: hash addresses printed with %p

[Joe Perches]

On Tue, 2017-10-31 at 09:33 +1100, Tobin C. Harding wrote:
> On Mon, Oct 30, 2017 at 03:03:21PM -0700, Kees Cook wrote:
> > On Wed, Oct 25, 2017 at 7:53 PM, Tobin C. Harding  wrote:
> > > Here is the behaviour that this set implements.
> > > 
> > > For kpt_restrict==0
> > > 
> > > Randomness not ready:
> > >   printed with %p:  (pointer)  # NOTE: with padding
> > > Valid pointer:
> > >   printed with %pK: deadbeefdeadbeef
> > >   printed with %p:  0xdeadbeef
> > >   malformed specifier (eg %i):  0xdeadbeef
> > 
> > I really think we can't include SPECIAL unless _every_ callsite of %p
> > is actually doing "0x%p", and then we're replacing all of those. We're
> > not doing that, though...
> > 
> > $ git grep '%p\b' | wc -l
> > 12766
> > $ git grep '0x%p\b' | wc -l
> > 18370x
> > 
> > If we need some kind of special marking that this is a hashed
> > variable, that should be something other than "0x". If we're using the
> > existing "(null)" and new "(pointer)" text, maybe "(hash:xx)"
> > should be used instead? Then the (rare) callers with 0x become
> > "0x(hash:)" and naked callers produce "(hash:)".
> > 
> > I think the first step for this is to just leave SPECIAL out.
> 
> Thanks Kees. V9 leaves SPECIAL out. Also V9 prints the whole 64 bit
> address with the first 32 bits masked to zero. The intent being to _not_
> change the output format from what it currently is. So it will look like
> this; 
> 
>   c09e81d0
> 
> What do you think?
> 
> Amusingly I think this whole conversation is going to come up again
> when we do %pa, in inverse, since %pa currently does us SPECIAL.

I once sent a patch set to remove SPECIAL from %pa
and add 0x where necessary.

https://patchwork.kernel.org/patch/3875471/

After that didn't happen, I removed the duplicated
0x%pa with a sed.

https://patchwork.kernel.org/patch/8509421/

Sending a treewide sed patch would be fine with me.

Re: [PATCH V8 0/2] printk: hash addresses printed with %p

[Joe Perches]

On Tue, 2017-10-31 at 09:33 +1100, Tobin C. Harding wrote:
> On Mon, Oct 30, 2017 at 03:03:21PM -0700, Kees Cook wrote:
> > On Wed, Oct 25, 2017 at 7:53 PM, Tobin C. Harding  wrote:
> > > Here is the behaviour that this set implements.
> > > 
> > > For kpt_restrict==0
> > > 
> > > Randomness not ready:
> > >   printed with %p:  (pointer)  # NOTE: with padding
> > > Valid pointer:
> > >   printed with %pK: deadbeefdeadbeef
> > >   printed with %p:  0xdeadbeef
> > >   malformed specifier (eg %i):  0xdeadbeef
> > 
> > I really think we can't include SPECIAL unless _every_ callsite of %p
> > is actually doing "0x%p", and then we're replacing all of those. We're
> > not doing that, though...
> > 
> > $ git grep '%p\b' | wc -l
> > 12766
> > $ git grep '0x%p\b' | wc -l
> > 18370x
> > 
> > If we need some kind of special marking that this is a hashed
> > variable, that should be something other than "0x". If we're using the
> > existing "(null)" and new "(pointer)" text, maybe "(hash:xx)"
> > should be used instead? Then the (rare) callers with 0x become
> > "0x(hash:)" and naked callers produce "(hash:)".
> > 
> > I think the first step for this is to just leave SPECIAL out.
> 
> Thanks Kees. V9 leaves SPECIAL out. Also V9 prints the whole 64 bit
> address with the first 32 bits masked to zero. The intent being to _not_
> change the output format from what it currently is. So it will look like
> this; 
> 
>   c09e81d0
> 
> What do you think?
> 
> Amusingly I think this whole conversation is going to come up again
> when we do %pa, in inverse, since %pa currently does us SPECIAL.

I once sent a patch set to remove SPECIAL from %pa
and add 0x where necessary.

https://patchwork.kernel.org/patch/3875471/

After that didn't happen, I removed the duplicated
0x%pa with a sed.

https://patchwork.kernel.org/patch/8509421/

Sending a treewide sed patch would be fine with me.

Re: [PATCH V8 0/2] printk: hash addresses printed with %p

[Tobin C. Harding]

On Mon, Oct 30, 2017 at 03:03:21PM -0700, Kees Cook wrote:
> On Wed, Oct 25, 2017 at 7:53 PM, Tobin C. Harding  wrote:
> > Here is the behaviour that this set implements.
> >
> > For kpt_restrict==0
> >
> > Randomness not ready:
> >   printed with %p:  (pointer)  # NOTE: with padding
> > Valid pointer:
> >   printed with %pK: deadbeefdeadbeef
> >   printed with %p:  0xdeadbeef
> >   malformed specifier (eg %i):  0xdeadbeef
> 
> I really think we can't include SPECIAL unless _every_ callsite of %p
> is actually doing "0x%p", and then we're replacing all of those. We're
> not doing that, though...
> 
> $ git grep '%p\b' | wc -l
> 12766
> $ git grep '0x%p\b' | wc -l
> 1837
> 
> If we need some kind of special marking that this is a hashed
> variable, that should be something other than "0x". If we're using the
> existing "(null)" and new "(pointer)" text, maybe "(hash:xx)"
> should be used instead? Then the (rare) callers with 0x become
> "0x(hash:)" and naked callers produce "(hash:)".
> 
> I think the first step for this is to just leave SPECIAL out.

Thanks Kees. V9 leaves SPECIAL out. Also V9 prints the whole 64 bit
address with the first 32 bits masked to zero. The intent being to _not_
change the output format from what it currently is. So it will look like
this; 

c09e81d0

What do you think?

Amusingly I think this whole conversation is going to come up again
when we do %pa, in inverse, since %pa currently does us SPECIAL.

thanks,
Tobin.

Re: [PATCH V8 0/2] printk: hash addresses printed with %p

[Tobin C. Harding]

On Mon, Oct 30, 2017 at 03:03:21PM -0700, Kees Cook wrote:
> On Wed, Oct 25, 2017 at 7:53 PM, Tobin C. Harding  wrote:
> > Here is the behaviour that this set implements.
> >
> > For kpt_restrict==0
> >
> > Randomness not ready:
> >   printed with %p:  (pointer)  # NOTE: with padding
> > Valid pointer:
> >   printed with %pK: deadbeefdeadbeef
> >   printed with %p:  0xdeadbeef
> >   malformed specifier (eg %i):  0xdeadbeef
> 
> I really think we can't include SPECIAL unless _every_ callsite of %p
> is actually doing "0x%p", and then we're replacing all of those. We're
> not doing that, though...
> 
> $ git grep '%p\b' | wc -l
> 12766
> $ git grep '0x%p\b' | wc -l
> 1837
> 
> If we need some kind of special marking that this is a hashed
> variable, that should be something other than "0x". If we're using the
> existing "(null)" and new "(pointer)" text, maybe "(hash:xx)"
> should be used instead? Then the (rare) callers with 0x become
> "0x(hash:)" and naked callers produce "(hash:)".
> 
> I think the first step for this is to just leave SPECIAL out.

Thanks Kees. V9 leaves SPECIAL out. Also V9 prints the whole 64 bit
address with the first 32 bits masked to zero. The intent being to _not_
change the output format from what it currently is. So it will look like
this; 

c09e81d0

What do you think?

Amusingly I think this whole conversation is going to come up again
when we do %pa, in inverse, since %pa currently does us SPECIAL.

thanks,
Tobin.

Re: [PATCH V8 0/2] printk: hash addresses printed with %p

[Kees Cook]

On Wed, Oct 25, 2017 at 7:53 PM, Tobin C. Harding  wrote:
> Here is the behaviour that this set implements.
>
> For kpt_restrict==0
>
> Randomness not ready:
>   printed with %p:  (pointer)  # NOTE: with padding
> Valid pointer:
>   printed with %pK: deadbeefdeadbeef
>   printed with %p:  0xdeadbeef
>   malformed specifier (eg %i):  0xdeadbeef

I really think we can't include SPECIAL unless _every_ callsite of %p
is actually doing "0x%p", and then we're replacing all of those. We're
not doing that, though...

$ git grep '%p\b' | wc -l
12766
$ git grep '0x%p\b' | wc -l
1837

If we need some kind of special marking that this is a hashed
variable, that should be something other than "0x". If we're using the
existing "(null)" and new "(pointer)" text, maybe "(hash:xx)"
should be used instead? Then the (rare) callers with 0x become
"0x(hash:)" and naked callers produce "(hash:)".

I think the first step for this is to just leave SPECIAL out.

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH V8 0/2] printk: hash addresses printed with %p

[Kees Cook]

On Wed, Oct 25, 2017 at 7:53 PM, Tobin C. Harding  wrote:
> Here is the behaviour that this set implements.
>
> For kpt_restrict==0
>
> Randomness not ready:
>   printed with %p:  (pointer)  # NOTE: with padding
> Valid pointer:
>   printed with %pK: deadbeefdeadbeef
>   printed with %p:  0xdeadbeef
>   malformed specifier (eg %i):  0xdeadbeef

I really think we can't include SPECIAL unless _every_ callsite of %p
is actually doing "0x%p", and then we're replacing all of those. We're
not doing that, though...

$ git grep '%p\b' | wc -l
12766
$ git grep '0x%p\b' | wc -l
1837

If we need some kind of special marking that this is a hashed
variable, that should be something other than "0x". If we're using the
existing "(null)" and new "(pointer)" text, maybe "(hash:xx)"
should be used instead? Then the (rare) callers with 0x become
"0x(hash:)" and naked callers produce "(hash:)".

I think the first step for this is to just leave SPECIAL out.

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH V8 0/2] printk: hash addresses printed with %p

[Sergey Senozhatsky]

On (10/26/17 13:53), Tobin C. Harding wrote:
> Currently there are many places in the kernel where addresses are being
> printed using an unadorned %p. Kernel pointers should be printed using
> %pK allowing some control via the kptr_restrict sysctl. Exposing
> addresses gives attackers sensitive information about the kernel layout
> in memory.
> 
> We can reduce the attack surface by hashing all addresses printed with
> %p. This will of course break some users, forcing code printing needed
> addresses to be updated.
> 
> With this version we include hashing of malformed specifiers also.
> Malformed specifiers include incomplete (e.g %pi) and also non-existent
> specifiers. checkpatch should warn for non-existent specifiers but
> AFAICT won't warn for incomplete specifiers.
> 
> Here is the behaviour that this set implements.
> 
> For kpt_restrict==0
> 
> Randomness not ready:
>   printed with %p:(pointer)  # NOTE: with padding
> Valid pointer:
>   printed with %pK:   deadbeefdeadbeef
>   printed with %p:0xdeadbeef
>   malformed specifier (eg %i):  0xdeadbeef
> NULL pointer:
>   printed with %pK:   
>   printed with %p:(null)   # NOTE: no padding
>   malformed specifier (eg %i):  (null)

a quick question:
 do we care about cases when kernel pointers are printed with %x/%X and
 not with %p?

-ss

Re: [PATCH V8 0/2] printk: hash addresses printed with %p

[Sergey Senozhatsky]

On (10/26/17 13:53), Tobin C. Harding wrote:
> Currently there are many places in the kernel where addresses are being
> printed using an unadorned %p. Kernel pointers should be printed using
> %pK allowing some control via the kptr_restrict sysctl. Exposing
> addresses gives attackers sensitive information about the kernel layout
> in memory.
> 
> We can reduce the attack surface by hashing all addresses printed with
> %p. This will of course break some users, forcing code printing needed
> addresses to be updated.
> 
> With this version we include hashing of malformed specifiers also.
> Malformed specifiers include incomplete (e.g %pi) and also non-existent
> specifiers. checkpatch should warn for non-existent specifiers but
> AFAICT won't warn for incomplete specifiers.
> 
> Here is the behaviour that this set implements.
> 
> For kpt_restrict==0
> 
> Randomness not ready:
>   printed with %p:(pointer)  # NOTE: with padding
> Valid pointer:
>   printed with %pK:   deadbeefdeadbeef
>   printed with %p:0xdeadbeef
>   malformed specifier (eg %i):  0xdeadbeef
> NULL pointer:
>   printed with %pK:   
>   printed with %p:(null)   # NOTE: no padding
>   malformed specifier (eg %i):  (null)

a quick question:
 do we care about cases when kernel pointers are printed with %x/%X and
 not with %p?

-ss

[PATCH V8 0/2] printk: hash addresses printed with %p

[Tobin C. Harding]

Currently there are many places in the kernel where addresses are being
printed using an unadorned %p. Kernel pointers should be printed using
%pK allowing some control via the kptr_restrict sysctl. Exposing
addresses gives attackers sensitive information about the kernel layout
in memory.

We can reduce the attack surface by hashing all addresses printed with
%p. This will of course break some users, forcing code printing needed
addresses to be updated.

With this version we include hashing of malformed specifiers also.
Malformed specifiers include incomplete (e.g %pi) and also non-existent
specifiers. checkpatch should warn for non-existent specifiers but
AFAICT won't warn for incomplete specifiers.

Here is the behaviour that this set implements.

For kpt_restrict==0

Randomness not ready:
  printed with %p:  (pointer)  # NOTE: with padding
Valid pointer:
  printed with %pK: deadbeefdeadbeef
  printed with %p:  0xdeadbeef
  malformed specifier (eg %i):  0xdeadbeef
NULL pointer:
  printed with %pK: 
  printed with %p:  (null)   # NOTE: no padding
  malformed specifier (eg %i):  (null)

For kpt_restrict==2

Valid pointer:
  printed with %pK: 

All other output as for kptr_restrict==0

V8:
 - Add second patch cleaning up null pointer printing in pointer()
 - Move %pK handling to separate function, further cleaning up pointer()
 - Move ptr_to_id() call outside of switch statement making hashing
   the default behaviour (including malformed specifiers).
 - Remove use of static_key, replace with simple boolean.

V7:
 - Use tabs instead of spaces (ouch!).

V6:
 - Use __early_initcall() to fill the SipHash key.
 - Use static keys to guard hashing before the key is available.

V5:
 - Remove spin lock.
 - Add Jason A. Donenfeld to CC list by request.
 - Add Theodore Ts'o to CC list due to comment on previous version.

V4:
 - Remove changes to siphash.{ch}
 - Do word size check, and return value cast, directly in ptr_to_id().
 - Use add_ready_random_callback() to guard call to get_random_bytes()

V3:
 - Use atomic_xchg() to guard setting [random] key.
 - Remove erroneous white space change.

V2:
 - Use SipHash to do the hashing.

The discussion related to this patch has been fragmented. There are
three threads associated with this patch. Email threads by subject:

[PATCH] printk: hash addresses printed with %p
[PATCH 0/3] add %pX specifier
[kernel-hardening] [RFC V2 0/6] add more kernel pointer filter options

Tobin C. Harding (2):
  printk: remove tabular output for NULL pointer
  printk: hash addresses printed with %p

 lib/vsprintf.c | 166 +
 1 file changed, 108 insertions(+), 58 deletions(-)

-- 
2.7.4

[PATCH V8 0/2] printk: hash addresses printed with %p

[Tobin C. Harding]

Currently there are many places in the kernel where addresses are being
printed using an unadorned %p. Kernel pointers should be printed using
%pK allowing some control via the kptr_restrict sysctl. Exposing
addresses gives attackers sensitive information about the kernel layout
in memory.

We can reduce the attack surface by hashing all addresses printed with
%p. This will of course break some users, forcing code printing needed
addresses to be updated.

With this version we include hashing of malformed specifiers also.
Malformed specifiers include incomplete (e.g %pi) and also non-existent
specifiers. checkpatch should warn for non-existent specifiers but
AFAICT won't warn for incomplete specifiers.

Here is the behaviour that this set implements.

For kpt_restrict==0

Randomness not ready:
  printed with %p:  (pointer)  # NOTE: with padding
Valid pointer:
  printed with %pK: deadbeefdeadbeef
  printed with %p:  0xdeadbeef
  malformed specifier (eg %i):  0xdeadbeef
NULL pointer:
  printed with %pK: 
  printed with %p:  (null)   # NOTE: no padding
  malformed specifier (eg %i):  (null)

For kpt_restrict==2

Valid pointer:
  printed with %pK: 

All other output as for kptr_restrict==0

V8:
 - Add second patch cleaning up null pointer printing in pointer()
 - Move %pK handling to separate function, further cleaning up pointer()
 - Move ptr_to_id() call outside of switch statement making hashing
   the default behaviour (including malformed specifiers).
 - Remove use of static_key, replace with simple boolean.

V7:
 - Use tabs instead of spaces (ouch!).

V6:
 - Use __early_initcall() to fill the SipHash key.
 - Use static keys to guard hashing before the key is available.

V5:
 - Remove spin lock.
 - Add Jason A. Donenfeld to CC list by request.
 - Add Theodore Ts'o to CC list due to comment on previous version.

V4:
 - Remove changes to siphash.{ch}
 - Do word size check, and return value cast, directly in ptr_to_id().
 - Use add_ready_random_callback() to guard call to get_random_bytes()

V3:
 - Use atomic_xchg() to guard setting [random] key.
 - Remove erroneous white space change.

V2:
 - Use SipHash to do the hashing.

The discussion related to this patch has been fragmented. There are
three threads associated with this patch. Email threads by subject:

[PATCH] printk: hash addresses printed with %p
[PATCH 0/3] add %pX specifier
[kernel-hardening] [RFC V2 0/6] add more kernel pointer filter options

Tobin C. Harding (2):
  printk: remove tabular output for NULL pointer
  printk: hash addresses printed with %p

 lib/vsprintf.c | 166 +
 1 file changed, 108 insertions(+), 58 deletions(-)

-- 
2.7.4