From 9f961c2e08741579aa53095d0dbffbcb25a9ae66 Mon Sep 17 00:00:00 2001 From: Marco Elver Date: Fri, 5 Mar 2021 20:42:05 +0100 Subject: lib/vsprintf: do not show no_hash_pointers message multiple times Do not show no_hash_pointers message multiple times if the option was passed more than once (e.g. via generated command line). Signed-off-by: Marco Elver Reviewed-by: Petr Mladek Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20210305194206.3165917-1-elver@google.com --- lib/vsprintf.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'lib') diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 41ddc353ebb8..4a14889ccb35 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -2096,6 +2096,9 @@ EXPORT_SYMBOL_GPL(no_hash_pointers); static int __init no_hash_pointers_enable(char *str) { + if (no_hash_pointers) + return 0; + no_hash_pointers = true; pr_warn("**********************************************************\n"); -- cgit v1.2.3 From c244297acbe51f1db5764966c02cdbd69927f218 Mon Sep 17 00:00:00 2001 From: Yafang Shao Date: Fri, 19 Mar 2021 18:12:46 +0800 Subject: vsprintf: dump full information of page flags in pGp Currently the pGp only shows the names of page flags, rather than the full information including section, node, zone, last cpupid and kasan tag. While it is not easy to parse these information manually because there're so many flavors. Let's interpret them in pGp as well. To be compitable with the existed format of pGp, the new introduced ones also use '|' as the separator, then the user tools parsing pGp won't need to make change, suggested by Matthew. The new information is tracked onto the end of the existed one. On example of the output in mm/slub.c as follows, - Before the patch, [ 6343.396602] Slab 0x000000004382e02b objects=33 used=3 fp=0x000000009ae06ffc flags=0x17ffffc0010200(slab|head) - After the patch, [ 8448.272530] Slab 0x0000000090797883 objects=33 used=3 fp=0x00000000790f1c26 flags=0x17ffffc0010200(slab|head|node=0|zone=2|lastcpupid=0x1fffff) The documentation and test cases are also updated. The output of the test cases as follows, [68599.816764] test_printf: loaded. [68599.819068] test_printf: all 388 tests passed [68599.830367] test_printf: unloaded. [lkp@intel.com: reported issues in the prev version in test_printf.c] Signed-off-by: Yafang Shao Cc: David Hildenbrand Cc: Joe Perches Cc: Miaohe Lin Cc: Vlastimil Babka Cc: Andy Shevchenko Cc: Matthew Wilcox Cc: Petr Mladek Cc: kernel test robot Reviewed-by: Sergey Senozhatsky Reviewed-by: Petr Mladek Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20210319101246.73513-4-laoar.shao@gmail.com --- Documentation/core-api/printk-formats.rst | 2 +- lib/test_printf.c | 90 +++++++++++++++++++++++++++---- lib/vsprintf.c | 66 +++++++++++++++++++++-- 3 files changed, 142 insertions(+), 16 deletions(-) (limited to 'lib') diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst index 160e710d992f..00d07c7eefd4 100644 --- a/Documentation/core-api/printk-formats.rst +++ b/Documentation/core-api/printk-formats.rst @@ -540,7 +540,7 @@ Flags bitfields such as page flags, gfp_flags :: - %pGp referenced|uptodate|lru|active|private + %pGp referenced|uptodate|lru|active|private|node=0|zone=2|lastcpupid=0x1fffff %pGg GFP_USER|GFP_DMA32|GFP_NOWARN %pGv read|exec|mayread|maywrite|mayexec|denywrite diff --git a/lib/test_printf.c b/lib/test_printf.c index 95a2f82427c7..27b964ec723d 100644 --- a/lib/test_printf.c +++ b/lib/test_printf.c @@ -577,24 +577,98 @@ netdev_features(void) { } +struct page_flags_test { + int width; + int shift; + int mask; + unsigned long value; + const char *fmt; + const char *name; +}; + +static struct page_flags_test pft[] = { + {SECTIONS_WIDTH, SECTIONS_PGSHIFT, SECTIONS_MASK, + 0, "%d", "section"}, + {NODES_WIDTH, NODES_PGSHIFT, NODES_MASK, + 0, "%d", "node"}, + {ZONES_WIDTH, ZONES_PGSHIFT, ZONES_MASK, + 0, "%d", "zone"}, + {LAST_CPUPID_WIDTH, LAST_CPUPID_PGSHIFT, LAST_CPUPID_MASK, + 0, "%#x", "lastcpupid"}, + {KASAN_TAG_WIDTH, KASAN_TAG_PGSHIFT, KASAN_TAG_MASK, + 0, "%#x", "kasantag"}, +}; + +static void __init +page_flags_test(int section, int node, int zone, int last_cpupid, + int kasan_tag, int flags, const char *name, char *cmp_buf) +{ + unsigned long values[] = {section, node, zone, last_cpupid, kasan_tag}; + unsigned long page_flags = 0; + unsigned long size = 0; + bool append = false; + int i; + + flags &= BIT(NR_PAGEFLAGS) - 1; + if (flags) { + page_flags |= flags; + snprintf(cmp_buf + size, BUF_SIZE - size, "%s", name); + size = strlen(cmp_buf); +#if SECTIONS_WIDTH || NODES_WIDTH || ZONES_WIDTH || \ + LAST_CPUPID_WIDTH || KASAN_TAG_WIDTH + /* Other information also included in page flags */ + snprintf(cmp_buf + size, BUF_SIZE - size, "|"); + size = strlen(cmp_buf); +#endif + } + + /* Set the test value */ + for (i = 0; i < ARRAY_SIZE(pft); i++) + pft[i].value = values[i]; + + for (i = 0; i < ARRAY_SIZE(pft); i++) { + if (!pft[i].width) + continue; + + if (append) { + snprintf(cmp_buf + size, BUF_SIZE - size, "|"); + size = strlen(cmp_buf); + } + + page_flags |= (pft[i].value & pft[i].mask) << pft[i].shift; + snprintf(cmp_buf + size, BUF_SIZE - size, "%s=", pft[i].name); + size = strlen(cmp_buf); + snprintf(cmp_buf + size, BUF_SIZE - size, pft[i].fmt, + pft[i].value & pft[i].mask); + size = strlen(cmp_buf); + append = true; + } + + test(cmp_buf, "%pGp", &page_flags); +} + static void __init flags(void) { unsigned long flags; - gfp_t gfp; char *cmp_buffer; + gfp_t gfp; + + cmp_buffer = kmalloc(BUF_SIZE, GFP_KERNEL); + if (!cmp_buffer) + return; flags = 0; - test("", "%pGp", &flags); + page_flags_test(0, 0, 0, 0, 0, flags, "", cmp_buffer); - /* Page flags should filter the zone id */ flags = 1UL << NR_PAGEFLAGS; - test("", "%pGp", &flags); + page_flags_test(0, 0, 0, 0, 0, flags, "", cmp_buffer); flags |= 1UL << PG_uptodate | 1UL << PG_dirty | 1UL << PG_lru | 1UL << PG_active | 1UL << PG_swapbacked; - test("uptodate|dirty|lru|active|swapbacked", "%pGp", &flags); - + page_flags_test(1, 1, 1, 0x1fffff, 1, flags, + "uptodate|dirty|lru|active|swapbacked", + cmp_buffer); flags = VM_READ | VM_EXEC | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC | VM_DENYWRITE; @@ -609,10 +683,6 @@ flags(void) gfp = __GFP_ATOMIC; test("__GFP_ATOMIC", "%pGg", &gfp); - cmp_buffer = kmalloc(BUF_SIZE, GFP_KERNEL); - if (!cmp_buffer) - return; - /* Any flags not translated by the table should remain numeric */ gfp = ~__GFP_BITS_MASK; snprintf(cmp_buffer, BUF_SIZE, "%#lx", (unsigned long) gfp); diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 41ddc353ebb8..92e6085eef15 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -1916,6 +1916,66 @@ char *format_flags(char *buf, char *end, unsigned long flags, return buf; } +struct page_flags_fields { + int width; + int shift; + int mask; + const struct printf_spec *spec; + const char *name; +}; + +static const struct page_flags_fields pff[] = { + {SECTIONS_WIDTH, SECTIONS_PGSHIFT, SECTIONS_MASK, + &default_dec_spec, "section"}, + {NODES_WIDTH, NODES_PGSHIFT, NODES_MASK, + &default_dec_spec, "node"}, + {ZONES_WIDTH, ZONES_PGSHIFT, ZONES_MASK, + &default_dec_spec, "zone"}, + {LAST_CPUPID_WIDTH, LAST_CPUPID_PGSHIFT, LAST_CPUPID_MASK, + &default_flag_spec, "lastcpupid"}, + {KASAN_TAG_WIDTH, KASAN_TAG_PGSHIFT, KASAN_TAG_MASK, + &default_flag_spec, "kasantag"}, +}; + +static +char *format_page_flags(char *buf, char *end, unsigned long flags) +{ + unsigned long main_flags = flags & (BIT(NR_PAGEFLAGS) - 1); + bool append = false; + int i; + + /* Page flags from the main area. */ + if (main_flags) { + buf = format_flags(buf, end, main_flags, pageflag_names); + append = true; + } + + /* Page flags from the fields area */ + for (i = 0; i < ARRAY_SIZE(pff); i++) { + /* Skip undefined fields. */ + if (!pff[i].width) + continue; + + /* Format: Flag Name + '=' (equals sign) + Number + '|' (separator) */ + if (append) { + if (buf < end) + *buf = '|'; + buf++; + } + + buf = string(buf, end, pff[i].name, default_str_spec); + if (buf < end) + *buf = '='; + buf++; + buf = number(buf, end, (flags >> pff[i].shift) & pff[i].mask, + *pff[i].spec); + + append = true; + } + + return buf; +} + static noinline_for_stack char *flags_string(char *buf, char *end, void *flags_ptr, struct printf_spec spec, const char *fmt) @@ -1928,11 +1988,7 @@ char *flags_string(char *buf, char *end, void *flags_ptr, switch (fmt[1]) { case 'p': - flags = *(unsigned long *)flags_ptr; - /* Remove zone id */ - flags &= (1UL << NR_PAGEFLAGS) - 1; - names = pageflag_names; - break; + return format_page_flags(buf, end, *(unsigned long *)flags_ptr); case 'v': flags = *(unsigned long *)flags_ptr; names = vmaflag_names; -- cgit v1.2.3 From a48849e2358ecf1a347a03b33dc27b9b2f25f8fd Mon Sep 17 00:00:00 2001 From: Vlastimil Babka Date: Thu, 25 Feb 2021 17:46:39 +0100 Subject: printk: clarify the documentation for plain pointer printing We have several modifiers for plain pointers (%p, %px and %pK) and now also the no_hash_pointers boot parameter. The documentation should help to choose which variant to use. Importantly, we should discourage %px in favor of %p (with the new boot parameter when debugging), and stress that %pK should be only used for procfs and similar files, not dmesg buffer. This patch clarifies the documentation in that regard. Signed-off-by: Vlastimil Babka Reviewed-by: Matthew Wilcox (Oracle) Reviewed-by: Petr Mladek Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20210225164639.27212-1-vbabka@suse.cz --- Documentation/core-api/printk-formats.rst | 26 +++++++++++++++++++++++++- lib/vsprintf.c | 7 +++++-- 2 files changed, 30 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst index 160e710d992f..6724adf58082 100644 --- a/Documentation/core-api/printk-formats.rst +++ b/Documentation/core-api/printk-formats.rst @@ -79,7 +79,19 @@ Pointers printed without a specifier extension (i.e unadorned %p) are hashed to prevent leaking information about the kernel memory layout. This has the added benefit of providing a unique identifier. On 64-bit machines the first 32 bits are zeroed. The kernel will print ``(ptrval)`` until it -gathers enough entropy. If you *really* want the address see %px below. +gathers enough entropy. + +When possible, use specialised modifiers such as %pS or %pB (described below) +to avoid the need of providing an unhashed address that has to be interpreted +post-hoc. If not possible, and the aim of printing the address is to provide +more information for debugging, use %p and boot the kernel with the +``no_hash_pointers`` parameter during debugging, which will print all %p +addresses unmodified. If you *really* always want the unmodified address, see +%px below. + +If (and only if) you are printing addresses as a content of a virtual file in +e.g. procfs or sysfs (using e.g. seq_printf(), not printk()) read by a +userspace process, use the %pK modifier described below instead of %p or %px. Error Pointers -------------- @@ -139,6 +151,11 @@ For printing kernel pointers which should be hidden from unprivileged users. The behaviour of %pK depends on the kptr_restrict sysctl - see Documentation/admin-guide/sysctl/kernel.rst for more details. +This modifier is *only* intended when producing content of a file read by +userspace from e.g. procfs or sysfs, not for dmesg. Please refer to the +section about %p above for discussion about how to manage hashing pointers +in printk(). + Unmodified Addresses -------------------- @@ -153,6 +170,13 @@ equivalent to %lx (or %lu). %px is preferred because it is more uniquely grep'able. If in the future we need to modify the way the kernel handles printing pointers we will be better equipped to find the call sites. +Before using %px, consider if using %p is sufficient together with enabling the +``no_hash_pointers`` kernel parameter during debugging sessions (see the %p +description above). One valid scenario for %px might be printing information +immediately before a panic, which prevents any sensitive information to be +exploited anyway, and with %px there would be no need to reproduce the panic +with no_hash_pointers. + Pointer Differences ------------------- diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 4a14889ccb35..5ec8ad238d03 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -2189,7 +2189,9 @@ early_param("no_hash_pointers", no_hash_pointers_enable); * Implements a "recursive vsnprintf". * Do not use this feature without some mechanism to verify the * correctness of the format string and va_list arguments. - * - 'K' For a kernel pointer that should be hidden from unprivileged users + * - 'K' For a kernel pointer that should be hidden from unprivileged users. + * Use only for procfs, sysfs and similar files, not printk(); please + * read the documentation (path below) first. * - 'NF' For a netdev_features_t * - 'h[CDN]' For a variable-length buffer, it prints it as a hex string with * a certain separator (' ' by default): @@ -2228,7 +2230,8 @@ early_param("no_hash_pointers", no_hash_pointers_enable); * Without an option prints the full name of the node * f full name * P node name, including a possible unit address - * - 'x' For printing the address. Equivalent to "%lx". + * - 'x' For printing the address unmodified. Equivalent to "%lx". + * Please read the documentation (path below) before using! * - '[ku]s' For a BPF/tracing related format specifier, e.g. used out of * bpf_trace_printk() where [ku] prefix specifies either kernel (k) * or user (u) memory to probe, and: -- cgit v1.2.3 From 84696cfaf4d90945eb2a8302edc6cf627db56b84 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Fri, 23 Apr 2021 11:45:29 +0200 Subject: lib/vsprintf.c: remove leftover 'f' and 'F' cases from bstr_printf() Commit 9af7706492f9 ("lib/vsprintf: Remove support for %pF and %pf in favour of %pS and %ps") removed support for %pF and %pf, and correctly removed the handling of those cases in vbin_printf(). However, the corresponding cases in bstr_printf() were left behind. In the same series, %pf was re-purposed for dealing with fwnodes (3bd32d6a2ee6, "lib/vsprintf: Add %pfw conversion specifier for printing fwnode names"). So should anyone use %pf with the binary printf routines, vbin_printf() would (correctly, as it involves dereferencing the pointer) do the string formatting to the u32 array, but bstr_printf() would not copy the string from the u32 array, but instead interpret the first sizeof(void*) bytes of the formatted string as a pointer - which generally won't end well (also, all subsequent get_args would be out of sync). Fixes: 9af7706492f9 ("lib/vsprintf: Remove support for %pF and %pf in favour of %pS and %ps") Cc: stable@vger.kernel.org Signed-off-by: Rasmus Villemoes Reviewed-by: Sakari Ailus Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20210423094529.1862521-1-linux@rasmusvillemoes.dk --- lib/vsprintf.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'lib') diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 5ec8ad238d03..7caf619ee9c2 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -3141,8 +3141,6 @@ int bstr_printf(char *buf, size_t size, const char *fmt, const u32 *bin_buf) switch (*fmt) { case 'S': case 's': - case 'F': - case 'f': case 'x': case 'K': case 'e': -- cgit v1.2.3