Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date: Wed, 11 Oct 2017 10:09:31 +1100
From: "Tobin C. Harding" <>
Cc: "Tobin C. Harding" <>,
	Linus Torvalds <>,
	Kees Cook <>,
	Paolo Bonzini <>,
	Tycho Andersen <>,
	"Roberts, William C" <>,
	Tejun Heo <>,
	Jordan Glover <>,
	Greg KH <>,
	Petr Mladek <>,
	Joe Perches <>,
	Ian Campbell <>,
	Sergey Senozhatsky <>,
	Catalin Marinas <>,
	Will Deacon <>,
	Steven Rostedt <>,
	Chris Fries <>,
	Dave Weinstein <>,
	Daniel Micay <>,
	Djalal Harouni <>
Subject: [PATCH 0/3] add %pX specifier

This series is a result of the recent thread on LKML regarding kpt_restrict

It seems we have not reached total consensus. This patch set does not claim to solve the whole issue
but rather take a small step forward without taking any steps backwards.

It may be that, since this issue is security related, there is no total solution only trade offs?

I am quite new to kernel development, which implies, neither am I a kernel security expert. In order
that my understanding of the issue is explicit I am listing here the things we all seem to agree on.

1. We are leaking addresses.

2. There are _some_ use cases for printing addresses.

3. Printing kernel pointers with %p and %x is bad.

4. We could reduce the number of leaked addresses if we had a mechanism to print unique identifiers.

If I am badly mistaken please feel free to yell at me, here to learn, happy to be corrected.

This patch set solves point 4 (above) by adding a printk specifier %pX to print a unique identifier
(hash) based on a pointer. This was suggested by Linus (in the above thread) as; 

  +        hashval = hash_three_words(
  +                (unsigned long)ptr,
  +                (unsigned long)ptr >> 16 >> 16,
  +                boot_time_random_int);

I did not understand the code (specifically why the right shift of 16 twice?). I therefore chose to
use an algorithm from kernel/kcmp.h for creating the hash (suggested by Tycho Anderson).

This patch is a softer version of Linus' suggestion because it does not change the behaviour of the
%p specifier. I don't see the benefit in making such a breaking change without addressing the issue
of %x (and I don't the balls to right now).

Patch 2 and 3 of the series give an example usage of the new specifier.

Thanks for taking the time to read this. All criticism and advice willingly accepted. 


Tobin C. Harding (3):
  lib/vsprintf: add 'X' specifier to hash pointers
  KVM: use %pX to print token identifier
  vfio_pci: use %pX to print token identifier

 Documentation/printk-formats.txt  |  9 +++++++++
 drivers/vfio/pci/vfio_pci_intrs.c |  2 +-
 include/linux/printk.h            | 17 +++++++++++++++++
 lib/vsprintf.c                    | 33 +++++++++++++++++++++++++++++++++
 scripts/             |  2 +-
 virt/kvm/eventfd.c                |  2 +-
 6 files changed, 62 insertions(+), 3 deletions(-)


Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.