From d75b26f880f60ead301e79ba0f4a635c5a60767f Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Thu, 27 Jan 2022 20:12:32 +0200 Subject: vsprintf: Fix potential unaligned access The %p4cc specifier in some cases might get an unaligned pointer. Due to this we need to make copy to local variable once to avoid potential crashes on some architectures due to improper access. Fixes: af612e43de6d ("lib/vsprintf: Add support for printing V4L2 and DRM fourccs") Cc: Sakari Ailus Signed-off-by: Andy Shevchenko Reviewed-by: Petr Mladek Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20220127181233.72910-1-andriy.shevchenko@linux.intel.com --- lib/vsprintf.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'lib/vsprintf.c') diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 53d6081f9e8b..3e416a3cb66d 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -49,6 +49,7 @@ #include /* for PAGE_SIZE */ #include /* cpu_to_le16 */ +#include #include #include "kstrtox.h" @@ -1771,7 +1772,7 @@ char *fourcc_string(char *buf, char *end, const u32 *fourcc, char output[sizeof("0123 little-endian (0x01234567)")]; char *p = output; unsigned int i; - u32 val; + u32 orig, val; if (fmt[1] != 'c' || fmt[2] != 'c') return error_string(buf, end, "(%p4?)", spec); @@ -1779,21 +1780,22 @@ char *fourcc_string(char *buf, char *end, const u32 *fourcc, if (check_pointer(&buf, end, fourcc, spec)) return buf; - val = *fourcc & ~BIT(31); + orig = get_unaligned(fourcc); + val = orig & ~BIT(31); - for (i = 0; i < sizeof(*fourcc); i++) { + for (i = 0; i < sizeof(u32); i++) { unsigned char c = val >> (i * 8); /* Print non-control ASCII characters as-is, dot otherwise */ *p++ = isascii(c) && isprint(c) ? c : '.'; } - strcpy(p, *fourcc & BIT(31) ? " big-endian" : " little-endian"); + strcpy(p, orig & BIT(31) ? " big-endian" : " little-endian"); p += strlen(p); *p++ = ' '; *p++ = '('; - p = special_hex_number(p, output + sizeof(output) - 2, *fourcc, sizeof(u32)); + p = special_hex_number(p, output + sizeof(output) - 2, orig, sizeof(u32)); *p++ = ')'; *p = '\0'; -- cgit From f74a08fc61073cc5b5f4e24eb513f0b79f4f6ce7 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Thu, 27 Jan 2022 20:12:33 +0200 Subject: vsprintf: Move space out of string literals in fourcc_string() The literals "big-endian" and "little-endian" may be potentially occurred in other places. Dropping space allows linker to merge them by using only a single copy. Signed-off-by: Andy Shevchenko Acked-by: Sakari Ailus Reviewed-by: Petr Mladek Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20220127181233.72910-2-andriy.shevchenko@linux.intel.com --- lib/vsprintf.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'lib/vsprintf.c') diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 3e416a3cb66d..f96c66899df6 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -1790,7 +1790,8 @@ char *fourcc_string(char *buf, char *end, const u32 *fourcc, *p++ = isascii(c) && isprint(c) ? c : '.'; } - strcpy(p, orig & BIT(31) ? " big-endian" : " little-endian"); + *p++ = ' '; + strcpy(p, orig & BIT(31) ? "big-endian" : "little-endian"); p += strlen(p); *p++ = ' '; -- cgit From 84842911322fc6a02a03ab9e728a48c691fe3efd Mon Sep 17 00:00:00 2001 From: Christophe Leroy Date: Thu, 17 Feb 2022 09:49:59 +0100 Subject: vsprintf: Fix %pK with kptr_restrict == 0 Although kptr_restrict is set to 0 and the kernel is booted with no_hash_pointers parameter, the content of /proc/vmallocinfo is lacking the real addresses. / # cat /proc/vmallocinfo 0x(ptrval)-0x(ptrval) 8192 load_module+0xc0c/0x2c0c pages=1 vmalloc 0x(ptrval)-0x(ptrval) 12288 start_kernel+0x4e0/0x690 pages=2 vmalloc 0x(ptrval)-0x(ptrval) 12288 start_kernel+0x4e0/0x690 pages=2 vmalloc 0x(ptrval)-0x(ptrval) 8192 _mpic_map_mmio.constprop.0+0x20/0x44 phys=0x80041000 ioremap 0x(ptrval)-0x(ptrval) 12288 _mpic_map_mmio.constprop.0+0x20/0x44 phys=0x80041000 ioremap ... According to the documentation for /proc/sys/kernel/, %pK is equivalent to %p when kptr_restrict is set to 0. Fixes: 5ead723a20e0 ("lib/vsprintf: no_hash_pointers prints all addresses as unhashed") Signed-off-by: Christophe Leroy Reviewed-by: Petr Mladek Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/107476128e59bff11a309b5bf7579a1753a41aca.1645087605.git.christophe.leroy@csgroup.eu --- lib/vsprintf.c | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-) (limited to 'lib/vsprintf.c') diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 53d6081f9e8b..8fb4a21c0b60 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -53,6 +53,10 @@ #include #include "kstrtox.h" +/* Disable pointer hashing if requested */ +bool no_hash_pointers __ro_after_init; +EXPORT_SYMBOL_GPL(no_hash_pointers); + static noinline unsigned long long simple_strntoull(const char *startp, size_t max_chars, char **endp, unsigned int base) { const char *cp; @@ -848,6 +852,19 @@ static char *ptr_to_id(char *buf, char *end, const void *ptr, return pointer_string(buf, end, (const void *)hashval, spec); } +static char *default_pointer(char *buf, char *end, const void *ptr, + struct printf_spec spec) +{ + /* + * default is to _not_ leak addresses, so hash before printing, + * unless no_hash_pointers is specified on the command line. + */ + if (unlikely(no_hash_pointers)) + return pointer_string(buf, end, ptr, spec); + + return ptr_to_id(buf, end, ptr, spec); +} + int kptr_restrict __read_mostly; static noinline_for_stack @@ -857,7 +874,7 @@ char *restricted_pointer(char *buf, char *end, const void *ptr, switch (kptr_restrict) { case 0: /* Handle as %p, hash and do _not_ leak addresses. */ - return ptr_to_id(buf, end, ptr, spec); + return default_pointer(buf, end, ptr, spec); case 1: { const struct cred *cred; @@ -2233,10 +2250,6 @@ char *fwnode_string(char *buf, char *end, struct fwnode_handle *fwnode, return widen_string(buf, buf - buf_start, end, spec); } -/* Disable pointer hashing if requested */ -bool no_hash_pointers __ro_after_init; -EXPORT_SYMBOL_GPL(no_hash_pointers); - int __init no_hash_pointers_enable(char *str) { if (no_hash_pointers) @@ -2465,7 +2478,7 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr, case 'e': /* %pe with a non-ERR_PTR gets treated as plain %p */ if (!IS_ERR(ptr)) - break; + return default_pointer(buf, end, ptr, spec); return err_ptr(buf, end, ptr, spec); case 'u': case 'k': @@ -2475,16 +2488,9 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr, default: return error_string(buf, end, "(einval)", spec); } + default: + return default_pointer(buf, end, ptr, spec); } - - /* - * default is to _not_ leak addresses, so hash before printing, - * unless no_hash_pointers is specified on the command line. - */ - if (unlikely(no_hash_pointers)) - return pointer_string(buf, end, ptr, spec); - else - return ptr_to_id(buf, end, ptr, spec); } /* -- cgit From 5acd35487dc911541672b3ffc322851769c32a56 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Tue, 1 Mar 2022 20:03:49 +0100 Subject: random: replace custom notifier chain with standard one We previously rolled our own randomness readiness notifier, which only has two users in the whole kernel. Replace this with a more standard atomic notifier block that serves the same purpose with less code. Also unexport the symbols, because no modules use it, only unconditional builtins. The only drawback is that it's possible for a notification handler returning the "stop" code to prevent further processing, but given that there are only two users, and that we're unexporting this anyway, that doesn't seem like a significant drawback for the simplification we receive here. Cc: Greg Kroah-Hartman Cc: Theodore Ts'o Reviewed-by: Dominik Brodowski Signed-off-by: Jason A. Donenfeld --- lib/vsprintf.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'lib/vsprintf.c') diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 3b8129dd374c..36574a806a81 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -757,14 +757,16 @@ static void enable_ptr_key_workfn(struct work_struct *work) static DECLARE_WORK(enable_ptr_key_work, enable_ptr_key_workfn); -static void fill_random_ptr_key(struct random_ready_callback *unused) +static int fill_random_ptr_key(struct notifier_block *nb, + unsigned long action, void *data) { /* This may be in an interrupt handler. */ queue_work(system_unbound_wq, &enable_ptr_key_work); + return 0; } -static struct random_ready_callback random_ready = { - .func = fill_random_ptr_key +static struct notifier_block random_ready = { + .notifier_call = fill_random_ptr_key }; static int __init initialize_ptr_random(void) @@ -778,7 +780,7 @@ static int __init initialize_ptr_random(void) return 0; } - ret = add_random_ready_callback(&random_ready); + ret = register_random_ready_notifier(&random_ready); if (!ret) { return 0; } else if (ret == -EALREADY) { -- cgit From ef62c8ff1de437ec2e0582205c1293c7dbbc4484 Mon Sep 17 00:00:00 2001 From: Waiman Long Date: Thu, 24 Mar 2022 18:09:02 -0700 Subject: lib/vsprintf: avoid redundant work with 0 size Patch series "mm/page_owner: Extend page_owner to show memcg information", v4. While debugging the constant increase in percpu memory consumption on a system that spawned large number of containers, it was found that a lot of offline mem_cgroup structures remained in place without being freed. Further investigation indicated that those mem_cgroup structures were pinned by some pages. In order to find out what those pages are, the existing page_owner debugging tool is extended to show memory cgroup information and whether those memcgs are offline or not. With the enhanced page_owner tool, the following is a typical page that pinned the mem_cgroup structure in my test case: Page allocated via order 0, mask 0x1100cca(GFP_HIGHUSER_MOVABLE), pid 162970 (podman), ts 1097761405537 ns, free_ts 1097760838089 ns PFN 1925700 type Movable Block 3761 type Movable Flags 0x17ffffc00c001c(uptodate|dirty|lru|reclaim|swapbacked|node=0|zone=2|lastcpupid=0x1fffff) prep_new_page+0xac/0xe0 get_page_from_freelist+0x1327/0x14d0 __alloc_pages+0x191/0x340 alloc_pages_vma+0x84/0x250 shmem_alloc_page+0x3f/0x90 shmem_alloc_and_acct_page+0x76/0x1c0 shmem_getpage_gfp+0x281/0x940 shmem_write_begin+0x36/0xe0 generic_perform_write+0xed/0x1d0 __generic_file_write_iter+0xdc/0x1b0 generic_file_write_iter+0x5d/0xb0 new_sync_write+0x11f/0x1b0 vfs_write+0x1ba/0x2a0 ksys_write+0x59/0xd0 do_syscall_64+0x37/0x80 entry_SYSCALL_64_after_hwframe+0x44/0xae Charged to offline memcg libpod-conmon-15e4f9c758422306b73b2dd99f9d50a5ea53cbb16b4a13a2c2308a4253cc0ec8. So the page was not freed because it was part of a shmem segment. That is useful information that can help users to diagnose similar problems. With cgroup v1, /proc/cgroups can be read to find out the total number of memory cgroups (online + offline). With cgroup v2, the cgroup.stat of the root cgroup can be read to find the number of dying cgroups (most likely pinned by dying memcgs). The page_owner feature is not supposed to be enabled for production system due to its memory overhead. However, if it is suspected that dying memcgs are increasing over time, a test environment with page_owner enabled can then be set up with appropriate workload for further analysis on what may be causing the increasing number of dying memcgs. This patch (of 4): For *scnprintf(), vsnprintf() is always called even if the input size is 0. That is a waste of time, so just return 0 in this case. Note that vsnprintf() will never return -1 to indicate an error. So skipping the call to vsnprintf() when size is 0 will have no functional impact at all. Link: https://lkml.kernel.org/r/20220202203036.744010-1-longman@redhat.com Link: https://lkml.kernel.org/r/20220202203036.744010-2-longman@redhat.com Signed-off-by: Waiman Long Acked-by: David Rientjes Reviewed-by: Sergey Senozhatsky Acked-by: Roman Gushchin Acked-by: Rafael Aquini Acked-by: Mike Rapoport Cc: Roman Gushchin Cc: Johannes Weiner Cc: Michal Hocko Cc: Vladimir Davydov Cc: Petr Mladek Cc: Steven Rostedt (Google) Cc: Andy Shevchenko Cc: Rasmus Villemoes Cc: Ira Weiny Cc: David Rientjes Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- lib/vsprintf.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'lib/vsprintf.c') diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 53fe73a48adf..40d26a07a133 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -2906,13 +2906,15 @@ int vscnprintf(char *buf, size_t size, const char *fmt, va_list args) { int i; + if (unlikely(!size)) + return 0; + i = vsnprintf(buf, size, fmt, args); if (likely(i < size)) return i; - if (size != 0) - return size - 1; - return 0; + + return size - 1; } EXPORT_SYMBOL(vscnprintf); -- cgit From 248561ad25a8ba4ecbc7df42f9a5a82fd5fbb4f6 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Sat, 14 May 2022 13:09:17 +0200 Subject: random: remove get_random_bytes_arch() and add rng_has_arch_random() The RNG incorporates RDRAND into its state at boot and every time it reseeds, so there's no reason for callers to use it directly. The hashing that the RNG does on it is preferable to using the bytes raw. The only current use case of get_random_bytes_arch() is vsprintf's siphash key for pointer hashing, which uses it to initialize the pointer secret earlier than usual if RDRAND is available. In order to replace this narrow use case, just expose whether RDRAND is mixed into the RNG, with a new function called rng_has_arch_random(). With that taken care of, there are no users of get_random_bytes_arch() left, so it can be removed. Later, if trust_cpu gets turned on by default (as most distros are doing), this one use of rng_has_arch_random() can probably go away as well. Cc: Steven Rostedt Cc: Sergey Senozhatsky Acked-by: Petr Mladek # for vsprintf.c Signed-off-by: Jason A. Donenfeld --- lib/vsprintf.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'lib/vsprintf.c') diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 40d26a07a133..20e9887faaaa 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -776,12 +776,11 @@ static struct notifier_block random_ready = { static int __init initialize_ptr_random(void) { - int key_size = sizeof(ptr_key); int ret; - /* Use hw RNG if available. */ - if (get_random_bytes_arch(&ptr_key, key_size) == key_size) { - static_branch_disable(¬_filled_random_ptr_key); + /* Don't bother waiting for RNG to be ready if RDRAND is mixed in already. */ + if (rng_has_arch_random()) { + enable_ptr_key_workfn(&enable_ptr_key_work); return 0; } -- cgit From 6701de6c51c172b5de5633374479503c81fefc0b Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Sun, 15 May 2022 15:06:18 +0200 Subject: random: remove mostly unused async readiness notifier The register_random_ready_notifier() notifier is somewhat complicated, and was already recently rewritten to use notifier blocks. It is only used now by one consumer in the kernel, vsprintf.c, for which the async mechanism is really overly complex for what it actually needs. This commit removes register_random_ready_notifier() and unregister_random_ ready_notifier(), because it just adds complication with little utility, and changes vsprintf.c to just check on `!rng_is_initialized() && !rng_has_arch_random()`, which will eventually be true. Performance- wise, that code was already using a static branch, so there's basically no overhead at all to this change. Cc: Steven Rostedt Cc: Sergey Senozhatsky Acked-by: Petr Mladek # for vsprintf.c Reviewed-by: Petr Mladek Signed-off-by: Jason A. Donenfeld --- lib/vsprintf.c | 66 ++++++++++++++++++++-------------------------------------- 1 file changed, 22 insertions(+), 44 deletions(-) (limited to 'lib/vsprintf.c') diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 20e9887faaaa..fb77f7bfd126 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -750,60 +750,38 @@ static int __init debug_boot_weak_hash_enable(char *str) } early_param("debug_boot_weak_hash", debug_boot_weak_hash_enable); -static DEFINE_STATIC_KEY_TRUE(not_filled_random_ptr_key); -static siphash_key_t ptr_key __read_mostly; +static DEFINE_STATIC_KEY_FALSE(filled_random_ptr_key); static void enable_ptr_key_workfn(struct work_struct *work) { - get_random_bytes(&ptr_key, sizeof(ptr_key)); - /* Needs to run from preemptible context */ - static_branch_disable(¬_filled_random_ptr_key); + static_branch_enable(&filled_random_ptr_key); } -static DECLARE_WORK(enable_ptr_key_work, enable_ptr_key_workfn); - -static int fill_random_ptr_key(struct notifier_block *nb, - unsigned long action, void *data) -{ - /* This may be in an interrupt handler. */ - queue_work(system_unbound_wq, &enable_ptr_key_work); - return 0; -} - -static struct notifier_block random_ready = { - .notifier_call = fill_random_ptr_key -}; - -static int __init initialize_ptr_random(void) -{ - int ret; - - /* Don't bother waiting for RNG to be ready if RDRAND is mixed in already. */ - if (rng_has_arch_random()) { - enable_ptr_key_workfn(&enable_ptr_key_work); - return 0; - } - - ret = register_random_ready_notifier(&random_ready); - if (!ret) { - return 0; - } else if (ret == -EALREADY) { - /* This is in preemptible context */ - enable_ptr_key_workfn(&enable_ptr_key_work); - return 0; - } - - return ret; -} -early_initcall(initialize_ptr_random); - /* Maps a pointer to a 32 bit unique identifier. */ static inline int __ptr_to_hashval(const void *ptr, unsigned long *hashval_out) { + static siphash_key_t ptr_key __read_mostly; unsigned long hashval; - if (static_branch_unlikely(¬_filled_random_ptr_key)) - return -EAGAIN; + if (!static_branch_likely(&filled_random_ptr_key)) { + static bool filled = false; + static DEFINE_SPINLOCK(filling); + static DECLARE_WORK(enable_ptr_key_work, enable_ptr_key_workfn); + unsigned long flags; + + if (!system_unbound_wq || + (!rng_is_initialized() && !rng_has_arch_random()) || + !spin_trylock_irqsave(&filling, flags)) + return -EAGAIN; + + if (!filled) { + get_random_bytes(&ptr_key, sizeof(ptr_key)); + queue_work(system_unbound_wq, &enable_ptr_key_work); + filled = true; + } + spin_unlock_irqrestore(&filling, flags); + } + #ifdef CONFIG_64BIT hashval = (unsigned long)siphash_1u64((u64)ptr, &ptr_key); -- cgit From e052a478a7daeca67664f7addd308ff51dd40654 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Wed, 8 Jun 2022 10:31:25 +0200 Subject: random: remove rng_has_arch_random() With arch randomness being used by every distro and enabled in defconfigs, the distinction between rng_has_arch_random() and rng_is_initialized() is now rather small. In fact, the places where they differ are now places where paranoid users and system builders really don't want arch randomness to be used, in which case we should respect that choice, or places where arch randomness is known to be broken, in which case that choice is all the more important. So this commit just removes the function and its one user. Reviewed-by: Petr Mladek # for vsprintf.c Signed-off-by: Jason A. Donenfeld --- lib/vsprintf.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'lib/vsprintf.c') diff --git a/lib/vsprintf.c b/lib/vsprintf.c index fb77f7bfd126..3c1853a9d1c0 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -769,8 +769,7 @@ static inline int __ptr_to_hashval(const void *ptr, unsigned long *hashval_out) static DECLARE_WORK(enable_ptr_key_work, enable_ptr_key_workfn); unsigned long flags; - if (!system_unbound_wq || - (!rng_is_initialized() && !rng_has_arch_random()) || + if (!system_unbound_wq || !rng_is_initialized() || !spin_trylock_irqsave(&filling, flags)) return -EAGAIN; -- cgit From dc453dd89daacdc0da6d66234aa27e417df7edcd Mon Sep 17 00:00:00 2001 From: Jian Shen Date: Tue, 16 Aug 2022 22:45:57 +0800 Subject: lib/vnsprintf: add const modifier for param 'bitmap' There is no modification for param bitmap in function bitmap_string() and bitmap_list_string(), so add const modifier for it. Signed-off-by: Jian Shen Signed-off-by: Guangbin Huang Reviewed-by: Steven Rostedt (Google) Reviewed-by: Sergey Senozhatsky Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20220816144557.30779-1-huangguangbin2@huawei.com --- lib/vsprintf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib/vsprintf.c') diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 3c1853a9d1c0..07b36d8b29c3 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -1189,7 +1189,7 @@ char *hex_string(char *buf, char *end, u8 *addr, struct printf_spec spec, } static noinline_for_stack -char *bitmap_string(char *buf, char *end, unsigned long *bitmap, +char *bitmap_string(char *buf, char *end, const unsigned long *bitmap, struct printf_spec spec, const char *fmt) { const int CHUNKSZ = 32; @@ -1233,7 +1233,7 @@ char *bitmap_string(char *buf, char *end, unsigned long *bitmap, } static noinline_for_stack -char *bitmap_list_string(char *buf, char *end, unsigned long *bitmap, +char *bitmap_list_string(char *buf, char *end, const unsigned long *bitmap, struct printf_spec spec, const char *fmt) { int nr_bits = max_t(int, spec.field_width, 0); -- cgit From 787983da77185d355564b0436f7b4eaa40b8904b Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Sat, 3 Jul 2021 17:38:57 +0200 Subject: vsprintf: add new `%pA` format specifier This patch adds a format specifier `%pA` to `vsprintf` which formats a pointer as `core::fmt::Arguments`. Doing so allows us to directly format to the internal buffer of `printf`, so we do not have to use a temporary buffer on the stack to pre-assemble the message on the Rust side. This specifier is intended only to be used from Rust and not for C, so `checkpatch.pl` is intentionally unchanged to catch any misuse. Reviewed-by: Kees Cook Acked-by: Petr Mladek Reviewed-by: Greg Kroah-Hartman Co-developed-by: Alex Gaynor Signed-off-by: Alex Gaynor Co-developed-by: Wedson Almeida Filho Signed-off-by: Wedson Almeida Filho Signed-off-by: Gary Guo Co-developed-by: Miguel Ojeda Signed-off-by: Miguel Ojeda --- lib/vsprintf.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'lib/vsprintf.c') diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 3c1853a9d1c0..c414a8d9f1ea 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -2246,6 +2246,9 @@ int __init no_hash_pointers_enable(char *str) } early_param("no_hash_pointers", no_hash_pointers_enable); +/* Used for Rust formatting ('%pA'). */ +char *rust_fmt_argument(char *buf, char *end, void *ptr); + /* * Show a '%p' thing. A kernel extension is that the '%p' is followed * by an extra set of alphanumeric characters that are extended format @@ -2372,6 +2375,10 @@ early_param("no_hash_pointers", no_hash_pointers_enable); * * Note: The default behaviour (unadorned %p) is to hash the address, * rendering it useful as a unique identifier. + * + * There is also a '%pA' format specifier, but it is only intended to be used + * from Rust code to format core::fmt::Arguments. Do *not* use it from C. + * See rust/kernel/print.rs for details. */ static noinline_for_stack char *pointer(const char *fmt, char *buf, char *end, void *ptr, @@ -2444,6 +2451,12 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr, return device_node_string(buf, end, ptr, spec, fmt + 1); case 'f': return fwnode_string(buf, end, ptr, spec, fmt + 1); + case 'A': + if (!IS_ENABLED(CONFIG_RUST)) { + WARN_ONCE(1, "Please remove %%pA from non-Rust code\n"); + return error_string(buf, end, "(%pA?)", spec); + } + return rust_fmt_argument(buf, end, ptr); case 'x': return pointer_string(buf, end, ptr, spec); case 'e': -- cgit From e4279b599863dd1aa71fb8e35bffa943545bbaeb Mon Sep 17 00:00:00 2001 From: Sebastian Andrzej Siewior Date: Tue, 27 Sep 2022 12:49:11 +0200 Subject: lib/vsprintf: Remove static_branch_likely() from __ptr_to_hashval(). Using static_branch_likely() to signal that ptr_key has been filled is a bit much given that it is not a fast path. Replace static_branch_likely() with bool for condition and a memory barrier for ptr_key. Suggested-by: Petr Mladek Signed-off-by: Sebastian Andrzej Siewior Reviewed-by: Petr Mladek Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20220927104912.622645-2-bigeasy@linutronix.de --- lib/vsprintf.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) (limited to 'lib/vsprintf.c') diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 3c1853a9d1c0..bce63cbf2377 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -750,12 +750,7 @@ static int __init debug_boot_weak_hash_enable(char *str) } early_param("debug_boot_weak_hash", debug_boot_weak_hash_enable); -static DEFINE_STATIC_KEY_FALSE(filled_random_ptr_key); - -static void enable_ptr_key_workfn(struct work_struct *work) -{ - static_branch_enable(&filled_random_ptr_key); -} +static bool filled_random_ptr_key __read_mostly; /* Maps a pointer to a 32 bit unique identifier. */ static inline int __ptr_to_hashval(const void *ptr, unsigned long *hashval_out) @@ -763,24 +758,26 @@ static inline int __ptr_to_hashval(const void *ptr, unsigned long *hashval_out) static siphash_key_t ptr_key __read_mostly; unsigned long hashval; - if (!static_branch_likely(&filled_random_ptr_key)) { + if (!READ_ONCE(filled_random_ptr_key)) { static bool filled = false; static DEFINE_SPINLOCK(filling); - static DECLARE_WORK(enable_ptr_key_work, enable_ptr_key_workfn); unsigned long flags; - if (!system_unbound_wq || !rng_is_initialized() || + if (!rng_is_initialized() || !spin_trylock_irqsave(&filling, flags)) return -EAGAIN; if (!filled) { get_random_bytes(&ptr_key, sizeof(ptr_key)); - queue_work(system_unbound_wq, &enable_ptr_key_work); + /* Pairs with smp_rmb() before reading ptr_key. */ + smp_wmb(); + WRITE_ONCE(filled_random_ptr_key, true); filled = true; } spin_unlock_irqrestore(&filling, flags); } - + /* Pairs with smp_wmb() after writing ptr_key. */ + smp_rmb(); #ifdef CONFIG_64BIT hashval = (unsigned long)siphash_1u64((u64)ptr, &ptr_key); -- cgit From 6f0ac3b52a9075b7291a72fb338d08491c1f0a64 Mon Sep 17 00:00:00 2001 From: Sebastian Andrzej Siewior Date: Tue, 27 Sep 2022 12:49:12 +0200 Subject: lib/vsprintf: Initialize vsprintf's pointer hash once the random core is ready. The printk code invokes vnsprintf in order to compute the complete string before adding it into its buffer. This happens in an IRQ-off region which leads to a warning on PREEMPT_RT in the random code if the format strings contains a %p for pointer printing. This happens because the random core acquires locks which become sleeping locks on PREEMPT_RT which must not be acquired with disabled interrupts and or preemption disabled. By default the pointers are hashed which requires a random value on the first invocation (either by printk or another user which comes first. One could argue that there is no need for printk to disable interrupts during the vsprintf() invocation which would fix the just mentioned problem. However printk itself can be invoked in a context with disabled interrupts which would lead to the very same problem. Move the initialization of ptr_key into a worker and schedule it from subsys_initcall(). This happens early but after the workqueue subsystem is ready. Use get_random_bytes() to retrieve the random value if the RNG core is ready, otherwise schedule a worker in two seconds and try again. Another advantage is that it removes a lock from the vsprintf() code path. It prevents a possible deadlock when printk("%p", ptr) is called under the lock taken in get_random_bytes(). Reported-by: Mike Galbraith Signed-off-by: Sebastian Andrzej Siewior Acked-by: Jason A. Donenfeld Reviewed-by: Petr Mladek [pmladek@suse.com: Added a note about the it prevented a possible deadlock in printk().] Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20220927104912.622645-3-bigeasy@linutronix.de --- lib/vsprintf.c | 46 +++++++++++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 19 deletions(-) (limited to 'lib/vsprintf.c') diff --git a/lib/vsprintf.c b/lib/vsprintf.c index bce63cbf2377..44b39ba56b79 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -751,31 +751,39 @@ static int __init debug_boot_weak_hash_enable(char *str) early_param("debug_boot_weak_hash", debug_boot_weak_hash_enable); static bool filled_random_ptr_key __read_mostly; +static siphash_key_t ptr_key __read_mostly; +static void fill_ptr_key_workfn(struct work_struct *work); +static DECLARE_DELAYED_WORK(fill_ptr_key_work, fill_ptr_key_workfn); + +static void fill_ptr_key_workfn(struct work_struct *work) +{ + if (!rng_is_initialized()) { + queue_delayed_work(system_unbound_wq, &fill_ptr_key_work, HZ * 2); + return; + } + + get_random_bytes(&ptr_key, sizeof(ptr_key)); + + /* Pairs with smp_rmb() before reading ptr_key. */ + smp_wmb(); + WRITE_ONCE(filled_random_ptr_key, true); +} + +static int __init vsprintf_init_hashval(void) +{ + fill_ptr_key_workfn(NULL); + return 0; +} +subsys_initcall(vsprintf_init_hashval) /* Maps a pointer to a 32 bit unique identifier. */ static inline int __ptr_to_hashval(const void *ptr, unsigned long *hashval_out) { - static siphash_key_t ptr_key __read_mostly; unsigned long hashval; - if (!READ_ONCE(filled_random_ptr_key)) { - static bool filled = false; - static DEFINE_SPINLOCK(filling); - unsigned long flags; - - if (!rng_is_initialized() || - !spin_trylock_irqsave(&filling, flags)) - return -EAGAIN; - - if (!filled) { - get_random_bytes(&ptr_key, sizeof(ptr_key)); - /* Pairs with smp_rmb() before reading ptr_key. */ - smp_wmb(); - WRITE_ONCE(filled_random_ptr_key, true); - filled = true; - } - spin_unlock_irqrestore(&filling, flags); - } + if (!READ_ONCE(filled_random_ptr_key)) + return -EBUSY; + /* Pairs with smp_wmb() after writing ptr_key. */ smp_rmb(); -- cgit