Commit da6af54d authored by Linus Torvalds's avatar Linus Torvalds

Merge tag 'printk-hash-pointer-4.15-rc2' of git://github.com/tcharding/linux

Pull printk pointer hashing update from Tobin Harding:
 "Here is the patch set that implements hashing of printk specifier %p.

  First we have two clean up patches then we do the hashing. Hashing is
  done via the SipHash algorithm. The next patch adds printk specifier
  %px for printing pointers when we _really_ want to see the address i.e
  %px is functionally equivalent to %lx. Final patch in the set fixes
  KASAN since we break it by hashing %p.

  For the record here is the justification for the series:

    Currently there exist approximately 14 000 places in the Kernel
    where addresses are being printed using an unadorned %p. This
    potentially leaks sensitive information about the Kernel layout in
    memory. Many of these calls are stale, instead of fixing every call
    we hash the address by default before printing. We then add %px to
    provide a way to print the actual address. Although this is
    achievable using %lx, using %px will assist us if we ever want to
    change pointer printing behaviour. %px is more uniquely grep'able
    (there are already >50 000 uses of %lx).

    The added advantage of hashing %p is that security is now opt-out,
    if you _really_ want the address you have to work a little harder
    and use %px.

  This will of course break some users, forcing code printing needed
  addresses to be updated"

[ I do expect this to be an annoyance, and a number of %px users to be
  added for debuggability. But nobody is willing to audit existing %p
  users for information leaks, and a number of places really only use
  the pointer as an object identifier rather than really 'I need the
  address'.

  IOW - sorry for the inconvenience, but it's the least inconvenient of
  the options.    - Linus ]

* tag 'printk-hash-pointer-4.15-rc2' of git://github.com/tcharding/linux:
  kasan: use %px to print addresses instead of %p
  vsprintf: add printk specifier %px
  printk: hash addresses printed with %p
  vsprintf: refactor %pK code out of pointer()
  docs: correct documentation for %pK
parents f55e1014 6424f6bb
...@@ -5,7 +5,6 @@ How to get printk format specifiers right ...@@ -5,7 +5,6 @@ How to get printk format specifiers right
:Author: Randy Dunlap <rdunlap@infradead.org> :Author: Randy Dunlap <rdunlap@infradead.org>
:Author: Andrew Murray <amurray@mpc-data.co.uk> :Author: Andrew Murray <amurray@mpc-data.co.uk>
Integer types Integer types
============= =============
...@@ -45,6 +44,18 @@ return from vsnprintf. ...@@ -45,6 +44,18 @@ return from vsnprintf.
Raw pointer value SHOULD be printed with %p. The kernel supports Raw pointer value SHOULD be printed with %p. The kernel supports
the following extended format specifiers for pointer types: the following extended format specifiers for pointer types:
Pointer Types
=============
Pointers printed without a specifier extension (i.e unadorned %p) are
hashed to give a unique identifier without leaking kernel addresses to user
space. On 64 bit machines the first 32 bits are zeroed. If you _really_
want the address see %px below.
::
%p abcdef12 or 00000000abcdef12
Symbols/Function Pointers Symbols/Function Pointers
========================= =========================
...@@ -85,18 +96,32 @@ Examples:: ...@@ -85,18 +96,32 @@ Examples::
printk("Faulted at %pS\n", (void *)regs->ip); printk("Faulted at %pS\n", (void *)regs->ip);
printk(" %s%pB\n", (reliable ? "" : "? "), (void *)*stack); printk(" %s%pB\n", (reliable ? "" : "? "), (void *)*stack);
Kernel Pointers Kernel Pointers
=============== ===============
:: ::
%pK 0x01234567 or 0x0123456789abcdef %pK 01234567 or 0123456789abcdef
For printing kernel pointers which should be hidden from unprivileged For printing kernel pointers which should be hidden from unprivileged
users. The behaviour of ``%pK`` depends on the ``kptr_restrict sysctl`` - see users. The behaviour of ``%pK`` depends on the ``kptr_restrict sysctl`` - see
Documentation/sysctl/kernel.txt for more details. Documentation/sysctl/kernel.txt for more details.
Unmodified Addresses
====================
::
%px 01234567 or 0123456789abcdef
For printing pointers when you _really_ want to print the address. Please
consider whether or not you are leaking sensitive information about the
Kernel layout in memory before printing pointers with %px. %px is
functionally equivalent to %lx. %px is preferred to %lx because it is more
uniquely grep'able. If, in the future, we need to modify the way the Kernel
handles printing pointers it will be nice to be able to find the call
sites.
Struct Resources Struct Resources
================ ================
......
...@@ -24,24 +24,6 @@ ...@@ -24,24 +24,6 @@
#define PAD_SIZE 16 #define PAD_SIZE 16
#define FILL_CHAR '$' #define FILL_CHAR '$'
#define PTR1 ((void*)0x01234567)
#define PTR2 ((void*)(long)(int)0xfedcba98)
#if BITS_PER_LONG == 64
#define PTR1_ZEROES "000000000"
#define PTR1_SPACES " "
#define PTR1_STR "1234567"
#define PTR2_STR "fffffffffedcba98"
#define PTR_WIDTH 16
#else
#define PTR1_ZEROES "0"
#define PTR1_SPACES " "
#define PTR1_STR "1234567"
#define PTR2_STR "fedcba98"
#define PTR_WIDTH 8
#endif
#define PTR_WIDTH_STR stringify(PTR_WIDTH)
static unsigned total_tests __initdata; static unsigned total_tests __initdata;
static unsigned failed_tests __initdata; static unsigned failed_tests __initdata;
static char *test_buffer __initdata; static char *test_buffer __initdata;
...@@ -217,30 +199,79 @@ test_string(void) ...@@ -217,30 +199,79 @@ test_string(void)
test("a | | ", "%-3.s|%-3.0s|%-3.*s", "a", "b", 0, "c"); test("a | | ", "%-3.s|%-3.0s|%-3.*s", "a", "b", 0, "c");
} }
#define PLAIN_BUF_SIZE 64 /* leave some space so we don't oops */
#if BITS_PER_LONG == 64
#define PTR_WIDTH 16
#define PTR ((void *)0xffff0123456789ab)
#define PTR_STR "ffff0123456789ab"
#define ZEROS "00000000" /* hex 32 zero bits */
static int __init
plain_format(void)
{
char buf[PLAIN_BUF_SIZE];
int nchars;
nchars = snprintf(buf, PLAIN_BUF_SIZE, "%p", PTR);
if (nchars != PTR_WIDTH || strncmp(buf, ZEROS, strlen(ZEROS)) != 0)
return -1;
return 0;
}
#else
#define PTR_WIDTH 8
#define PTR ((void *)0x456789ab)
#define PTR_STR "456789ab"
static int __init
plain_format(void)
{
/* Format is implicitly tested for 32 bit machines by plain_hash() */
return 0;
}
#endif /* BITS_PER_LONG == 64 */
static int __init
plain_hash(void)
{
char buf[PLAIN_BUF_SIZE];
int nchars;
nchars = snprintf(buf, PLAIN_BUF_SIZE, "%p", PTR);
if (nchars != PTR_WIDTH || strncmp(buf, PTR_STR, PTR_WIDTH) == 0)
return -1;
return 0;
}
/*
* We can't use test() to test %p because we don't know what output to expect
* after an address is hashed.
*/
static void __init static void __init
plain(void) plain(void)
{ {
test(PTR1_ZEROES PTR1_STR " " PTR2_STR, "%p %p", PTR1, PTR2); int err;
/*
* The field width is overloaded for some %p extensions to
* pass another piece of information. For plain pointers, the
* behaviour is slightly odd: One cannot pass either the 0
* flag nor a precision to %p without gcc complaining, and if
* one explicitly gives a field width, the number is no longer
* zero-padded.
*/
test("|" PTR1_STR PTR1_SPACES " | " PTR1_SPACES PTR1_STR "|",
"|%-*p|%*p|", PTR_WIDTH+2, PTR1, PTR_WIDTH+2, PTR1);
test("|" PTR2_STR " | " PTR2_STR "|",
"|%-*p|%*p|", PTR_WIDTH+2, PTR2, PTR_WIDTH+2, PTR2);
/* err = plain_hash();
* Unrecognized %p extensions are treated as plain %p, but the if (err) {
* alphanumeric suffix is ignored (that is, does not occur in pr_warn("plain 'p' does not appear to be hashed\n");
* the output.) failed_tests++;
*/ return;
test("|"PTR1_ZEROES PTR1_STR"|", "|%p0y|", PTR1); }
test("|"PTR2_STR"|", "|%p0y|", PTR2);
err = plain_format();
if (err) {
pr_warn("hashing plain 'p' has unexpected format\n");
failed_tests++;
}
} }
static void __init static void __init
...@@ -251,6 +282,7 @@ symbol_ptr(void) ...@@ -251,6 +282,7 @@ symbol_ptr(void)
static void __init static void __init
kernel_ptr(void) kernel_ptr(void)
{ {
/* We can't test this without access to kptr_restrict. */
} }
static void __init static void __init
......
...@@ -33,6 +33,8 @@ ...@@ -33,6 +33,8 @@
#include <linux/uuid.h> #include <linux/uuid.h>
#include <linux/of.h> #include <linux/of.h>
#include <net/addrconf.h> #include <net/addrconf.h>
#include <linux/siphash.h>
#include <linux/compiler.h>
#ifdef CONFIG_BLOCK #ifdef CONFIG_BLOCK
#include <linux/blkdev.h> #include <linux/blkdev.h>
#endif #endif
...@@ -1343,6 +1345,59 @@ char *uuid_string(char *buf, char *end, const u8 *addr, ...@@ -1343,6 +1345,59 @@ char *uuid_string(char *buf, char *end, const u8 *addr,
return string(buf, end, uuid, spec); return string(buf, end, uuid, spec);
} }
int kptr_restrict __read_mostly;
static noinline_for_stack
char *restricted_pointer(char *buf, char *end, const void *ptr,
struct printf_spec spec)
{
spec.base = 16;
spec.flags |= SMALL;
if (spec.field_width == -1) {
spec.field_width = 2 * sizeof(ptr);
spec.flags |= ZEROPAD;
}
switch (kptr_restrict) {
case 0:
/* Always print %pK values */
break;
case 1: {
const struct cred *cred;
/*
* kptr_restrict==1 cannot be used in IRQ context
* because its test for CAP_SYSLOG would be meaningless.
*/
if (in_irq() || in_serving_softirq() || in_nmi())
return string(buf, end, "pK-error", spec);
/*
* Only print the real pointer value if the current
* process has CAP_SYSLOG and is running with the
* same credentials it started with. This is because
* access to files is checked at open() time, but %pK
* checks permission at read() time. We don't want to
* leak pointer values if a binary opens a file using
* %pK and then elevates privileges before reading it.
*/
cred = current_cred();
if (!has_capability_noaudit(current, CAP_SYSLOG) ||
!uid_eq(cred->euid, cred->uid) ||
!gid_eq(cred->egid, cred->gid))
ptr = NULL;
break;
}
case 2:
default:
/* Always print 0's for %pK */
ptr = NULL;
break;
}
return number(buf, end, (unsigned long)ptr, spec);
}
static noinline_for_stack static noinline_for_stack
char *netdev_bits(char *buf, char *end, const void *addr, const char *fmt) char *netdev_bits(char *buf, char *end, const void *addr, const char *fmt)
{ {
...@@ -1591,7 +1646,86 @@ char *device_node_string(char *buf, char *end, struct device_node *dn, ...@@ -1591,7 +1646,86 @@ char *device_node_string(char *buf, char *end, struct device_node *dn,
return widen_string(buf, buf - buf_start, end, spec); return widen_string(buf, buf - buf_start, end, spec);
} }
int kptr_restrict __read_mostly; static noinline_for_stack
char *pointer_string(char *buf, char *end, const void *ptr,
struct printf_spec spec)
{
spec.base = 16;
spec.flags |= SMALL;
if (spec.field_width == -1) {
spec.field_width = 2 * sizeof(ptr);
spec.flags |= ZEROPAD;
}
return number(buf, end, (unsigned long int)ptr, spec);
}
static bool have_filled_random_ptr_key __read_mostly;
static siphash_key_t ptr_key __read_mostly;
static void fill_random_ptr_key(struct random_ready_callback *unused)
{
get_random_bytes(&ptr_key, sizeof(ptr_key));
/*
* have_filled_random_ptr_key==true is dependent on get_random_bytes().
* ptr_to_id() needs to see have_filled_random_ptr_key==true
* after get_random_bytes() returns.
*/
smp_mb();
WRITE_ONCE(have_filled_random_ptr_key, true);
}
static struct random_ready_callback random_ready = {
.func = fill_random_ptr_key
};
static int __init initialize_ptr_random(void)
{
int ret = add_random_ready_callback(&random_ready);
if (!ret) {
return 0;
} else if (ret == -EALREADY) {
fill_random_ptr_key(&random_ready);
return 0;
}
return ret;
}
early_initcall(initialize_ptr_random);
/* Maps a pointer to a 32 bit unique identifier. */
static char *ptr_to_id(char *buf, char *end, void *ptr, struct printf_spec spec)
{
unsigned long hashval;
const int default_width = 2 * sizeof(ptr);
if (unlikely(!have_filled_random_ptr_key)) {
spec.field_width = default_width;
/* string length must be less than default_width */
return string(buf, end, "(ptrval)", spec);
}
#ifdef CONFIG_64BIT
hashval = (unsigned long)siphash_1u64((u64)ptr, &ptr_key);
/*
* Mask off the first 32 bits, this makes explicit that we have
* modified the address (and 32 bits is plenty for a unique ID).
*/
hashval = hashval & 0xffffffff;
#else
hashval = (unsigned long)siphash_1u32((u32)ptr, &ptr_key);
#endif
spec.flags |= SMALL;
if (spec.field_width == -1) {
spec.field_width = default_width;
spec.flags |= ZEROPAD;
}
spec.base = 16;
return number(buf, end, hashval, spec);
}
/* /*
* Show a '%p' thing. A kernel extension is that the '%p' is followed * Show a '%p' thing. A kernel extension is that the '%p' is followed
...@@ -1698,11 +1832,16 @@ int kptr_restrict __read_mostly; ...@@ -1698,11 +1832,16 @@ int kptr_restrict __read_mostly;
* c major compatible string * c major compatible string
* C full compatible string * C full compatible string
* *
* - 'x' For printing the address. Equivalent to "%lx".
*
* ** Please update also Documentation/printk-formats.txt when making changes ** * ** Please update also Documentation/printk-formats.txt when making changes **
* *
* Note: The difference between 'S' and 'F' is that on ia64 and ppc64 * Note: The difference between 'S' and 'F' is that on ia64 and ppc64
* function pointers are really function descriptors, which contain a * function pointers are really function descriptors, which contain a
* pointer to the real address. * pointer to the real address.
*
* Note: The default behaviour (unadorned %p) is to hash the address,
* rendering it useful as a unique identifier.
*/ */
static noinline_for_stack static noinline_for_stack
char *pointer(const char *fmt, char *buf, char *end, void *ptr, char *pointer(const char *fmt, char *buf, char *end, void *ptr,
...@@ -1792,47 +1931,7 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr, ...@@ -1792,47 +1931,7 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
return buf; return buf;
} }
case 'K': case 'K':
switch (kptr_restrict) { return restricted_pointer(buf, end, ptr, spec);
case 0:
/* Always print %pK values */
break;
case 1: {
const struct cred *cred;
/*
* kptr_restrict==1 cannot be used in IRQ context
* because its test for CAP_SYSLOG would be meaningless.
*/
if (in_irq() || in_serving_softirq() || in_nmi()) {
if (spec.field_width == -1)
spec.field_width = default_width;
return string(buf, end, "pK-error", spec);
}
/*
* Only print the real pointer value if the current
* process has CAP_SYSLOG and is running with the
* same credentials it started with. This is because
* access to files is checked at open() time, but %pK
* checks permission at read() time. We don't want to
* leak pointer values if a binary opens a file using
* %pK and then elevates privileges before reading it.
*/
cred = current_cred();
if (!has_capability_noaudit(current, CAP_SYSLOG) ||
!uid_eq(cred->euid, cred->uid) ||
!gid_eq(cred->egid, cred->gid))
ptr = NULL;
break;
}
case 2:
default:
/* Always print 0's for %pK */
ptr = NULL;
break;
}
break;
case 'N': case 'N':
return netdev_bits(buf, end, ptr, fmt); return netdev_bits(buf, end, ptr, fmt);
case 'a': case 'a':
...@@ -1857,15 +1956,12 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr, ...@@ -1857,15 +1956,12 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
case 'F': case 'F':
return device_node_string(buf, end, ptr, spec, fmt + 1); return device_node_string(buf, end, ptr, spec, fmt + 1);
} }
case 'x':
return pointer_string(buf, end, ptr, spec);
} }
spec.flags |= SMALL;
if (spec.field_width == -1) {
spec.field_width = default_width;
spec.flags |= ZEROPAD;
}
spec.base = 16;
return number(buf, end, (unsigned long) ptr, spec); /* default is to _not_ leak addresses, hash before printing */
return ptr_to_id(buf, end, ptr, spec);
} }
/* /*
......
...@@ -134,7 +134,7 @@ static void print_error_description(struct kasan_access_info *info) ...@@ -134,7 +134,7 @@ static void print_error_description(struct kasan_access_info *info)
pr_err("BUG: KASAN: %s in %pS\n", pr_err("BUG: KASAN: %s in %pS\n",
bug_type, (void *)info->ip); bug_type, (void *)info->ip);
pr_err("%s of size %zu at addr %p by task %s/%d\n", pr_err("%s of size %zu at addr %px by task %s/%d\n",
info->is_write ? "Write" : "Read", info->access_size, info->is_write ? "Write" : "Read", info->access_size,
info->access_addr, current->comm, task_pid_nr(current)); info->access_addr, current->comm, task_pid_nr(current));
} }
...@@ -206,7 +206,7 @@ static void describe_object_addr(struct kmem_cache *cache, void *object, ...@@ -206,7 +206,7 @@ static void describe_object_addr(struct kmem_cache *cache, void *object,
const char *rel_type; const char *rel_type;
int rel_bytes; int rel_bytes;
pr_err("The buggy address belongs to the object at %p\n" pr_err("The buggy address belongs to the object at %px\n"
" which belongs to the cache %s of size %d\n", " which belongs to the cache %s of size %d\n",
object, cache->name, cache->object_size); object, cache->name, cache->object_size);
...@@ -225,7 +225,7 @@ static void describe_object_addr(struct kmem_cache *cache, void *object, ...@@ -225,7 +225,7 @@ static void describe_object_addr(struct kmem_cache *cache, void *object,
} }
pr_err("The buggy address is located %d bytes %s of\n" pr_err("The buggy address is located %d bytes %s of\n"
" %d-byte region [%p, %p)\n", " %d-byte region [%px, %px)\n",
rel_bytes, rel_type, cache->object_size, (void *)object_addr, rel_bytes, rel_type, cache->object_size, (void *)object_addr,
(void *)(object_addr + cache->object_size)); (void *)(object_addr + cache->object_size));
} }
...@@ -302,7 +302,7 @@ static void print_shadow_for_address(const void *addr) ...@@ -302,7 +302,7 @@ static void print_shadow_for_address(const void *addr)
char shadow_buf[SHADOW_BYTES_PER_ROW]; char shadow_buf[SHADOW_BYTES_PER_ROW];
snprintf(buffer, sizeof(buffer), snprintf(buffer, sizeof(buffer),
(i == 0) ? ">%p: " : " %p: ", kaddr); (i == 0) ? ">%px: " : " %px: ", kaddr);
/* /*
* We should not pass a shadow pointer to generic * We should not pass a shadow pointer to generic
* function, because generic functions may try to * function, because generic functions may try to
......
...@@ -5753,7 +5753,7 @@ sub process { ...@@ -5753,7 +5753,7 @@ sub process {
for (my $count = $linenr; $count <= $lc; $count++) { for (my $count = $linenr; $count <= $lc; $count++) {
my $fmt = get_quoted_string($lines[$count - 1], raw_line($count, 0)); my $fmt = get_quoted_string($lines[$count - 1], raw_line($count, 0));
$fmt =~ s/%%//g; $fmt =~ s/%%//g;
if ($fmt =~ /(\%[\*\d\.]*p(?![\WFfSsBKRraEhMmIiUDdgVCbGNO]).)/) { if ($fmt =~ /(\%[\*\d\.]*p(?![\WFfSsBKRraEhMmIiUDdgVCbGNOx]).)/) {
$bad_extension = $1; $bad_extension = $1;
last; last;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment