Message ID | 20210224200101.9835-2-tim.gardner@canonical.com |
---|---|
State | New |
Headers | show |
Series | CVE-2018-7273 | expand |
On Wed, Feb 24, 2021 at 01:01:01PM -0700, Tim Gardner wrote: > From: "Tobin C. Harding" <me@tobin.cc> > > Currently there exist approximately 14 000 places in the kernel where > addresses are being printed using an unadorned %p. This potentially > leaks sensitive information regarding the Kernel layout in memory. Many > of these calls are stale, instead of fixing every call lets hash the > address by default before printing. This will of course break some > users, forcing code printing needed addresses to be updated. > > Code that _really_ needs the address will soon be able to use the new > printk specifier %px to print the address. > > For what it's worth, usage of unadorned %p can be broken down as > follows (thanks to Joe Perches). > > $ git grep -E '%p[^A-Za-z0-9]' | cut -f1 -d"/" | sort | uniq -c > 1084 arch > 20 block > 10 crypto > 32 Documentation > 8121 drivers > 1221 fs > 143 include > 101 kernel > 69 lib > 100 mm > 1510 net > 40 samples > 7 scripts > 11 security > 166 sound > 152 tools > 2 virt > > Add function ptr_to_id() to map an address to a 32 bit unique > identifier. Hash any unadorned usage of specifier %p and any malformed > specifiers. > > Signed-off-by: Tobin C. Harding <me@tobin.cc> > (backported from commit ad67b74d2469d9b82aaa572d76474c95bc484d57) > CVE-2018-7273 > Signed-off-by: Tim Gardner <tim.gardner@canonical.com> > > Conflicts: > Documentation/printk-formats.txt (Added a paragraph under Kernel Pointers) > lib/vsprintf.c (required additional include files, no code changes) > --- > Documentation/printk-formats.txt | 6 ++ > lib/test_printf.c | 108 ++++++++++++++++++++----------- > lib/vsprintf.c | 86 ++++++++++++++++++++++-- > 3 files changed, 155 insertions(+), 45 deletions(-) > > diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.txt > index ed6f6abaad57..2329b1eac0fa 100644 > --- a/Documentation/printk-formats.txt > +++ b/Documentation/printk-formats.txt > @@ -64,6 +64,12 @@ Kernel Pointers: > users. The behaviour of %pK depends on the kptr_restrict sysctl - see > Documentation/sysctl/kernel.txt for more details. > > + 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. > + > + %p abcdef12 or 00000000abcdef12 > + > Struct Resources: > > %pr [mem 0x60000000-0x6fffffff flags 0x2200] or > diff --git a/lib/test_printf.c b/lib/test_printf.c > index c5a666af9ba5..e2200f06f168 100644 > --- a/lib/test_printf.c > +++ b/lib/test_printf.c > @@ -18,24 +18,6 @@ > #define BUF_SIZE 256 > #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 failed_tests __initdata; > static char *test_buffer __initdata; > @@ -160,30 +142,79 @@ test_string(void) > 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 > plain(void) > { > - test(PTR1_ZEROES PTR1_STR " " PTR2_STR, "%p %p", PTR1, PTR2); > - /* > - * 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); > + int err; > > - /* > - * Unrecognized %p extensions are treated as plain %p, but the > - * alphanumeric suffix is ignored (that is, does not occur in > - * the output.) > - */ > - test("|"PTR1_ZEROES PTR1_STR"|", "|%p0y|", PTR1); > - test("|"PTR2_STR"|", "|%p0y|", PTR2); > + err = plain_hash(); > + if (err) { > + pr_warn("plain 'p' does not appear to be hashed\n"); > + failed_tests++; > + return; > + } > + > + err = plain_format(); > + if (err) { > + pr_warn("hashing plain 'p' has unexpected format\n"); > + failed_tests++; > + } > } > > static void __init > @@ -194,6 +225,7 @@ symbol_ptr(void) > static void __init > kernel_ptr(void) > { > + /* We can't test this without access to kptr_restrict. */ > } > > static void __init > diff --git a/lib/vsprintf.c b/lib/vsprintf.c > index 646009db4198..3cfeeaf0518d 100644 > --- a/lib/vsprintf.c > +++ b/lib/vsprintf.c > @@ -31,6 +31,13 @@ > #include <linux/dcache.h> > #include <linux/cred.h> > #include <net/addrconf.h> > +#include <linux/siphash.h> > +#include <linux/compiler.h> > +#ifdef CONFIG_BLOCK > +#include <linux/blkdev.h> > +#endif > + > +#include "../mm/internal.h" /* For the trace_print_flags arrays */ > This hunk is not needed from what I could see from history. It's worth build testing anyway, but they should be left out. Can you also test that it works as expected in respect to dmesg and kptr_restrict values, specially for the earliest messages, as they depend on random to be initialized? Cascardo. > #include <asm/page.h> /* for PAGE_SIZE */ > #include <asm/sections.h> /* for dereference_function_descriptor() */ > @@ -1360,6 +1367,73 @@ char *clock(char *buf, char *end, struct clk *clk, struct printf_spec spec, > > int kptr_restrict __read_mostly; > > +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 > * by an extra set of alphanumeric characters that are extended format > @@ -1451,6 +1525,9 @@ int kptr_restrict __read_mostly; > * Note: The difference between 'S' and 'F' is that on ia64 and ppc64 > * function pointers are really function descriptors, which contain a > * 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 > char *pointer(const char *fmt, char *buf, char *end, void *ptr, > @@ -1598,14 +1675,9 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr, > ((const struct file *)ptr)->f_path.dentry, > spec, fmt); > } > - 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); > } > > /* > -- > 2.17.1 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
On 2/24/21 1:26 PM, Thadeu Lima de Souza Cascardo wrote: > On Wed, Feb 24, 2021 at 01:01:01PM -0700, Tim Gardner wrote: >> From: "Tobin C. Harding" <me@tobin.cc> >> >> Currently there exist approximately 14 000 places in the kernel where >> addresses are being printed using an unadorned %p. This potentially >> leaks sensitive information regarding the Kernel layout in memory. Many >> of these calls are stale, instead of fixing every call lets hash the >> address by default before printing. This will of course break some >> users, forcing code printing needed addresses to be updated. >> >> Code that _really_ needs the address will soon be able to use the new >> printk specifier %px to print the address. >> >> For what it's worth, usage of unadorned %p can be broken down as >> follows (thanks to Joe Perches). >> >> $ git grep -E '%p[^A-Za-z0-9]' | cut -f1 -d"/" | sort | uniq -c >> 1084 arch >> 20 block >> 10 crypto >> 32 Documentation >> 8121 drivers >> 1221 fs >> 143 include >> 101 kernel >> 69 lib >> 100 mm >> 1510 net >> 40 samples >> 7 scripts >> 11 security >> 166 sound >> 152 tools >> 2 virt >> >> Add function ptr_to_id() to map an address to a 32 bit unique >> identifier. Hash any unadorned usage of specifier %p and any malformed >> specifiers. >> >> Signed-off-by: Tobin C. Harding <me@tobin.cc> >> (backported from commit ad67b74d2469d9b82aaa572d76474c95bc484d57) >> CVE-2018-7273 >> Signed-off-by: Tim Gardner <tim.gardner@canonical.com> >> >> Conflicts: >> Documentation/printk-formats.txt (Added a paragraph under Kernel Pointers) >> lib/vsprintf.c (required additional include files, no code changes) >> --- >> Documentation/printk-formats.txt | 6 ++ >> lib/test_printf.c | 108 ++++++++++++++++++++----------- >> lib/vsprintf.c | 86 ++++++++++++++++++++++-- >> 3 files changed, 155 insertions(+), 45 deletions(-) >> >> diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.txt >> index ed6f6abaad57..2329b1eac0fa 100644 >> --- a/Documentation/printk-formats.txt >> +++ b/Documentation/printk-formats.txt >> @@ -64,6 +64,12 @@ Kernel Pointers: >> users. The behaviour of %pK depends on the kptr_restrict sysctl - see >> Documentation/sysctl/kernel.txt for more details. >> >> + 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. >> + >> + %p abcdef12 or 00000000abcdef12 >> + >> Struct Resources: >> >> %pr [mem 0x60000000-0x6fffffff flags 0x2200] or >> diff --git a/lib/test_printf.c b/lib/test_printf.c >> index c5a666af9ba5..e2200f06f168 100644 >> --- a/lib/test_printf.c >> +++ b/lib/test_printf.c >> @@ -18,24 +18,6 @@ >> #define BUF_SIZE 256 >> #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 failed_tests __initdata; >> static char *test_buffer __initdata; >> @@ -160,30 +142,79 @@ test_string(void) >> 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 >> plain(void) >> { >> - test(PTR1_ZEROES PTR1_STR " " PTR2_STR, "%p %p", PTR1, PTR2); >> - /* >> - * 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); >> + int err; >> >> - /* >> - * Unrecognized %p extensions are treated as plain %p, but the >> - * alphanumeric suffix is ignored (that is, does not occur in >> - * the output.) >> - */ >> - test("|"PTR1_ZEROES PTR1_STR"|", "|%p0y|", PTR1); >> - test("|"PTR2_STR"|", "|%p0y|", PTR2); >> + err = plain_hash(); >> + if (err) { >> + pr_warn("plain 'p' does not appear to be hashed\n"); >> + failed_tests++; >> + return; >> + } >> + >> + err = plain_format(); >> + if (err) { >> + pr_warn("hashing plain 'p' has unexpected format\n"); >> + failed_tests++; >> + } >> } >> >> static void __init >> @@ -194,6 +225,7 @@ symbol_ptr(void) >> static void __init >> kernel_ptr(void) >> { >> + /* We can't test this without access to kptr_restrict. */ >> } >> >> static void __init >> diff --git a/lib/vsprintf.c b/lib/vsprintf.c >> index 646009db4198..3cfeeaf0518d 100644 >> --- a/lib/vsprintf.c >> +++ b/lib/vsprintf.c >> @@ -31,6 +31,13 @@ >> #include <linux/dcache.h> >> #include <linux/cred.h> >> #include <net/addrconf.h> >> +#include <linux/siphash.h> >> +#include <linux/compiler.h> > > >> +#ifdef CONFIG_BLOCK >> +#include <linux/blkdev.h> >> +#endif >> + >> +#include "../mm/internal.h" /* For the trace_print_flags arrays */ >> > > This hunk is not needed from what I could see from history. It's worth build > testing anyway, but they should be left out. > Indeed, those include files are not required. > Can you also test that it works as expected in respect to dmesg and > kptr_restrict values, specially for the earliest messages, as they depend on > random to be initialized? > The %pK format (kptr_restrict) is natively supported in Xenial. One has to assume upstream thought about when kptr_restrict!=0 could be used with respect to when /dev/random is initialized. The use of an unadorned %p format early in boot is a little tougher to predict wrt randomness. However, even if /dev/random isn't initialized, we're no worse off then we were before. Right ? rtg > Cascardo. > >> #include <asm/page.h> /* for PAGE_SIZE */ >> #include <asm/sections.h> /* for dereference_function_descriptor() */ >> @@ -1360,6 +1367,73 @@ char *clock(char *buf, char *end, struct clk *clk, struct printf_spec spec, >> >> int kptr_restrict __read_mostly; >> >> +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 >> * by an extra set of alphanumeric characters that are extended format >> @@ -1451,6 +1525,9 @@ int kptr_restrict __read_mostly; >> * Note: The difference between 'S' and 'F' is that on ia64 and ppc64 >> * function pointers are really function descriptors, which contain a >> * 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 >> char *pointer(const char *fmt, char *buf, char *end, void *ptr, >> @@ -1598,14 +1675,9 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr, >> ((const struct file *)ptr)->f_path.dentry, >> spec, fmt); >> } >> - 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); >> } >> >> /* >> -- >> 2.17.1 >> >> >> -- >> kernel-team mailing list >> kernel-team@lists.ubuntu.com >> https://lists.ubuntu.com/mailman/listinfo/kernel-team
diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.txt index ed6f6abaad57..2329b1eac0fa 100644 --- a/Documentation/printk-formats.txt +++ b/Documentation/printk-formats.txt @@ -64,6 +64,12 @@ Kernel Pointers: users. The behaviour of %pK depends on the kptr_restrict sysctl - see Documentation/sysctl/kernel.txt for more details. + 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. + + %p abcdef12 or 00000000abcdef12 + Struct Resources: %pr [mem 0x60000000-0x6fffffff flags 0x2200] or diff --git a/lib/test_printf.c b/lib/test_printf.c index c5a666af9ba5..e2200f06f168 100644 --- a/lib/test_printf.c +++ b/lib/test_printf.c @@ -18,24 +18,6 @@ #define BUF_SIZE 256 #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 failed_tests __initdata; static char *test_buffer __initdata; @@ -160,30 +142,79 @@ test_string(void) 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 plain(void) { - test(PTR1_ZEROES PTR1_STR " " PTR2_STR, "%p %p", PTR1, PTR2); - /* - * 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); + int err; - /* - * Unrecognized %p extensions are treated as plain %p, but the - * alphanumeric suffix is ignored (that is, does not occur in - * the output.) - */ - test("|"PTR1_ZEROES PTR1_STR"|", "|%p0y|", PTR1); - test("|"PTR2_STR"|", "|%p0y|", PTR2); + err = plain_hash(); + if (err) { + pr_warn("plain 'p' does not appear to be hashed\n"); + failed_tests++; + return; + } + + err = plain_format(); + if (err) { + pr_warn("hashing plain 'p' has unexpected format\n"); + failed_tests++; + } } static void __init @@ -194,6 +225,7 @@ symbol_ptr(void) static void __init kernel_ptr(void) { + /* We can't test this without access to kptr_restrict. */ } static void __init diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 646009db4198..3cfeeaf0518d 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -31,6 +31,13 @@ #include <linux/dcache.h> #include <linux/cred.h> #include <net/addrconf.h> +#include <linux/siphash.h> +#include <linux/compiler.h> +#ifdef CONFIG_BLOCK +#include <linux/blkdev.h> +#endif + +#include "../mm/internal.h" /* For the trace_print_flags arrays */ #include <asm/page.h> /* for PAGE_SIZE */ #include <asm/sections.h> /* for dereference_function_descriptor() */ @@ -1360,6 +1367,73 @@ char *clock(char *buf, char *end, struct clk *clk, struct printf_spec spec, int kptr_restrict __read_mostly; +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 * by an extra set of alphanumeric characters that are extended format @@ -1451,6 +1525,9 @@ int kptr_restrict __read_mostly; * Note: The difference between 'S' and 'F' is that on ia64 and ppc64 * function pointers are really function descriptors, which contain a * 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 char *pointer(const char *fmt, char *buf, char *end, void *ptr, @@ -1598,14 +1675,9 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr, ((const struct file *)ptr)->f_path.dentry, spec, fmt); } - 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); } /*