From af1da686843750809738c01e153320106e890804 Mon Sep 17 00:00:00 2001 From: Miles Chen Date: Thu, 22 Feb 2018 19:22:20 +0800 Subject: dma-debug: fix memory leak in debug_dma_alloc_coherent Marty reported a memory leakage introduced by commit 3aaabbf1c39e ("lib/dma-debug.c: fix incorrect pfn calculation"). Fix it by checking the virtual address before allocating the entry. This patch also use virt_addr_valid() instead of virt_to_page() to check if a virtual address is linear. Fixes: 3aaabbf1 ("lib/dma-debug.c: fix incorrect pfn calculation") Reported-by: Marty Faltesek Signed-off-by: Miles Chen Acked-by: Robin Murphy Signed-off-by: Christoph Hellwig --- lib/dma-debug.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'lib') diff --git a/lib/dma-debug.c b/lib/dma-debug.c index 1b34d210452c..7f5cdc1e6b29 100644 --- a/lib/dma-debug.c +++ b/lib/dma-debug.c @@ -1491,12 +1491,12 @@ void debug_dma_alloc_coherent(struct device *dev, size_t size, if (unlikely(virt == NULL)) return; - entry = dma_entry_alloc(); - if (!entry) + /* handle vmalloc and linear addresses */ + if (!is_vmalloc_addr(virt) && !virt_addr_valid(virt)) return; - /* handle vmalloc and linear addresses */ - if (!is_vmalloc_addr(virt) && !virt_to_page(virt)) + entry = dma_entry_alloc(); + if (!entry) return; entry->type = dma_debug_coherent; @@ -1528,7 +1528,7 @@ void debug_dma_free_coherent(struct device *dev, size_t size, }; /* handle vmalloc and linear addresses */ - if (!is_vmalloc_addr(virt) && !virt_to_page(virt)) + if (!is_vmalloc_addr(virt) && !virt_addr_valid(virt)) return; if (is_vmalloc_addr(virt)) -- cgit v1.2.3-58-ga151 From 4b0ad07653ee94182e2d8f21404242c9e83ad0b4 Mon Sep 17 00:00:00 2001 From: Matthew Wilcox Date: Mon, 26 Feb 2018 14:39:30 -0500 Subject: idr: Fix handling of IDs above INT_MAX Khalid reported that the kernel selftests are currently failing: selftests: test_bpf.sh ======================================== test_bpf: [FAIL] not ok 1..8 selftests: test_bpf.sh [FAIL] He bisected it to 6ce711f2750031d12cec91384ac5cfa0a485b60a ("idr: Make 1-based IDRs more efficient"). The root cause is doing a signed comparison in idr_alloc_u32() instead of an unsigned comparison. I went looking for any similar problems and found a couple (which would each result in the failure to warn in two situations that aren't supposed to happen). I knocked up a few test-cases to prove that I was right and added them to the test-suite. Reported-by: Khalid Aziz Tested-by: Khalid Aziz Signed-off-by: Matthew Wilcox --- lib/idr.c | 13 +++++----- tools/testing/radix-tree/idr-test.c | 52 +++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 6 deletions(-) (limited to 'lib') diff --git a/lib/idr.c b/lib/idr.c index 99ec5bc89d25..823b813f08f8 100644 --- a/lib/idr.c +++ b/lib/idr.c @@ -36,8 +36,8 @@ int idr_alloc_u32(struct idr *idr, void *ptr, u32 *nextid, { struct radix_tree_iter iter; void __rcu **slot; - int base = idr->idr_base; - int id = *nextid; + unsigned int base = idr->idr_base; + unsigned int id = *nextid; if (WARN_ON_ONCE(radix_tree_is_internal_node(ptr))) return -EINVAL; @@ -204,10 +204,11 @@ int idr_for_each(const struct idr *idr, radix_tree_for_each_slot(slot, &idr->idr_rt, &iter, 0) { int ret; + unsigned long id = iter.index + base; - if (WARN_ON_ONCE(iter.index > INT_MAX)) + if (WARN_ON_ONCE(id > INT_MAX)) break; - ret = fn(iter.index + base, rcu_dereference_raw(*slot), data); + ret = fn(id, rcu_dereference_raw(*slot), data); if (ret) return ret; } @@ -230,8 +231,8 @@ void *idr_get_next(struct idr *idr, int *nextid) { struct radix_tree_iter iter; void __rcu **slot; - int base = idr->idr_base; - int id = *nextid; + unsigned long base = idr->idr_base; + unsigned long id = *nextid; id = (id < base) ? 0 : id - base; slot = radix_tree_iter_find(&idr->idr_rt, &iter, id); diff --git a/tools/testing/radix-tree/idr-test.c b/tools/testing/radix-tree/idr-test.c index 44ef9eba5a7a..6c645eb77d42 100644 --- a/tools/testing/radix-tree/idr-test.c +++ b/tools/testing/radix-tree/idr-test.c @@ -178,6 +178,55 @@ void idr_get_next_test(int base) idr_destroy(&idr); } +int idr_u32_cb(int id, void *ptr, void *data) +{ + BUG_ON(id < 0); + BUG_ON(ptr != DUMMY_PTR); + return 0; +} + +void idr_u32_test1(struct idr *idr, u32 handle) +{ + static bool warned = false; + u32 id = handle; + int sid = 0; + void *ptr; + + BUG_ON(idr_alloc_u32(idr, DUMMY_PTR, &id, id, GFP_KERNEL)); + BUG_ON(id != handle); + BUG_ON(idr_alloc_u32(idr, DUMMY_PTR, &id, id, GFP_KERNEL) != -ENOSPC); + BUG_ON(id != handle); + if (!warned && id > INT_MAX) + printk("vvv Ignore these warnings\n"); + ptr = idr_get_next(idr, &sid); + if (id > INT_MAX) { + BUG_ON(ptr != NULL); + BUG_ON(sid != 0); + } else { + BUG_ON(ptr != DUMMY_PTR); + BUG_ON(sid != id); + } + idr_for_each(idr, idr_u32_cb, NULL); + if (!warned && id > INT_MAX) { + printk("^^^ Warnings over\n"); + warned = true; + } + BUG_ON(idr_remove(idr, id) != DUMMY_PTR); + BUG_ON(!idr_is_empty(idr)); +} + +void idr_u32_test(int base) +{ + DEFINE_IDR(idr); + idr_init_base(&idr, base); + idr_u32_test1(&idr, 10); + idr_u32_test1(&idr, 0x7fffffff); + idr_u32_test1(&idr, 0x80000000); + idr_u32_test1(&idr, 0x80000001); + idr_u32_test1(&idr, 0xffe00000); + idr_u32_test1(&idr, 0xffffffff); +} + void idr_checks(void) { unsigned long i; @@ -248,6 +297,9 @@ void idr_checks(void) idr_get_next_test(0); idr_get_next_test(1); idr_get_next_test(4); + idr_u32_test(4); + idr_u32_test(1); + idr_u32_test(0); } /* -- cgit v1.2.3-58-ga151 From d40bc96257fe070796c63934913f95cc183016b0 Mon Sep 17 00:00:00 2001 From: Eric Dumazet Date: Mon, 26 Feb 2018 10:52:46 -0800 Subject: test_bpf: add a schedule point test_bpf() is taking 1.6 seconds nowadays, it is time to add a schedule point in it. Signed-off-by: Eric Dumazet Signed-off-by: Daniel Borkmann --- lib/test_bpf.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'lib') diff --git a/lib/test_bpf.c b/lib/test_bpf.c index b4e22345963f..e6f550608d72 100644 --- a/lib/test_bpf.c +++ b/lib/test_bpf.c @@ -24,6 +24,7 @@ #include #include #include +#include /* General test specific settings */ #define MAX_SUBTESTS 3 @@ -6582,6 +6583,7 @@ static __init int test_bpf(void) struct bpf_prog *fp; int err; + cond_resched(); if (exclude_test(i)) continue; -- cgit v1.2.3-58-ga151 From 9960d7669eaa42e82a2f4393adf549191de2e587 Mon Sep 17 00:00:00 2001 From: Eric Dumazet Date: Wed, 28 Feb 2018 08:39:20 -0800 Subject: test_bpf: reduce MAX_TESTRUNS For tests that are using the maximal number of BPF instruction, each run takes 20 usec. Looping 10,000 times on them totals 200 ms, which is bad when the loop is not preemptible. test_bpf: #264 BPF_MAXINSNS: Call heavy transformations jited:1 19248 18548 PASS test_bpf: #269 BPF_MAXINSNS: ld_abs+get_processor_id jited:1 20896 PASS Lets divide by ten the number of iterations, so that max latency is 20ms. We could use need_resched() to break the loop earlier if we believe 20 ms is too much. Signed-off-by: Eric Dumazet Signed-off-by: Daniel Borkmann --- lib/test_bpf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/test_bpf.c b/lib/test_bpf.c index e6f550608d72..2efb213716fa 100644 --- a/lib/test_bpf.c +++ b/lib/test_bpf.c @@ -28,7 +28,7 @@ /* General test specific settings */ #define MAX_SUBTESTS 3 -#define MAX_TESTRUNS 10000 +#define MAX_TESTRUNS 1000 #define MAX_DATA 128 #define MAX_INSNS 512 #define MAX_K 0xffffFFFF -- cgit v1.2.3-58-ga151 From 0862ca422b79cb5aa70823ee0f07f6b468f86070 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 9 Mar 2018 15:50:59 -0800 Subject: bug: use %pB in BUG and stack protector failure The BUG and stack protector reports were still using a raw %p. This changes it to %pB for more meaningful output. Link: http://lkml.kernel.org/r/20180301225704.GA34198@beast Fixes: ad67b74d2469 ("printk: hash addresses printed with %p") Signed-off-by: Kees Cook Reviewed-by: Andrew Morton Cc: Ingo Molnar Cc: Thomas Gleixner Cc: Peter Zijlstra Cc: Borislav Petkov Cc: Richard Weinberger , Cc: Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- kernel/panic.c | 2 +- lib/bug.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/kernel/panic.c b/kernel/panic.c index 2cfef408fec9..4b794f1d8561 100644 --- a/kernel/panic.c +++ b/kernel/panic.c @@ -640,7 +640,7 @@ device_initcall(register_warn_debugfs); */ __visible void __stack_chk_fail(void) { - panic("stack-protector: Kernel stack is corrupted in: %p\n", + panic("stack-protector: Kernel stack is corrupted in: %pB\n", __builtin_return_address(0)); } EXPORT_SYMBOL(__stack_chk_fail); diff --git a/lib/bug.c b/lib/bug.c index c1b0fad31b10..44f432cb064d 100644 --- a/lib/bug.c +++ b/lib/bug.c @@ -191,7 +191,7 @@ enum bug_trap_type report_bug(unsigned long bugaddr, struct pt_regs *regs) if (file) pr_crit("kernel BUG at %s:%u!\n", file, line); else - pr_crit("Kernel BUG at %p [verbose debug info unavailable]\n", + pr_crit("Kernel BUG at %pB [verbose debug info unavailable]\n", (void *)bugaddr); return BUG_TRAP_TYPE_BUG; -- cgit v1.2.3-58-ga151 From 1b4cfe3c0a30dde968fb43c577a8d7e262a145ee Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 9 Mar 2018 15:51:02 -0800 Subject: lib/bug.c: exclude non-BUG/WARN exceptions from report_bug() Commit b8347c219649 ("x86/debug: Handle warnings before the notifier chain, to fix KGDB crash") changed the ordering of fixups, and did not take into account the case of x86 processing non-WARN() and non-BUG() exceptions. This would lead to output of a false BUG line with no other information. In the case of a refcount exception, it would be immediately followed by the refcount WARN(), producing very strange double-"cut here": lkdtm: attempting bad refcount_inc() overflow ------------[ cut here ]------------ Kernel BUG at 0000000065f29de5 [verbose debug info unavailable] ------------[ cut here ]------------ refcount_t overflow at lkdtm_REFCOUNT_INC_OVERFLOW+0x6b/0x90 in cat[3065], uid/euid: 0/0 WARNING: CPU: 0 PID: 3065 at kernel/panic.c:657 refcount_error_report+0x9a/0xa4 ... In the prior ordering, exceptions were searched first: do_trap_no_signal(struct task_struct *tsk, int trapnr, char *str, ... if (fixup_exception(regs, trapnr)) return 0; - if (fixup_bug(regs, trapnr)) - return 0; - As a result, fixup_bugs()'s is_valid_bugaddr() didn't take into account needing to search the exception list first, since that had already happened. So, instead of searching the exception list twice (once in is_valid_bugaddr() and then again in fixup_exception()), just add a simple sanity check to report_bug() that will immediately bail out if a BUG() (or WARN()) entry is not found. Link: http://lkml.kernel.org/r/20180301225934.GA34350@beast Fixes: b8347c219649 ("x86/debug: Handle warnings before the notifier chain, to fix KGDB crash") Signed-off-by: Kees Cook Cc: Ingo Molnar Cc: Thomas Gleixner Cc: Peter Zijlstra Cc: Borislav Petkov Cc: Richard Weinberger Cc: Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- lib/bug.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'lib') diff --git a/lib/bug.c b/lib/bug.c index 44f432cb064d..1077366f496b 100644 --- a/lib/bug.c +++ b/lib/bug.c @@ -150,6 +150,8 @@ enum bug_trap_type report_bug(unsigned long bugaddr, struct pt_regs *regs) return BUG_TRAP_TYPE_NONE; bug = find_bug(bugaddr); + if (!bug) + return BUG_TRAP_TYPE_NONE; file = NULL; line = 0; -- cgit v1.2.3-58-ga151 From ac68b1b3b9c73e652dc7ce0585672e23c5a2dca4 Mon Sep 17 00:00:00 2001 From: "Luis R. Rodriguez" Date: Fri, 9 Mar 2018 15:51:20 -0800 Subject: lib/test_kmod.c: fix limit check on number of test devices created As reported by Dan the parentheses is in the wrong place, and since unlikely() call returns either 0 or 1 it's never less than zero. The second issue is that signed integer overflows like "INT_MAX + 1" are undefined behavior. Since num_test_devs represents the number of devices, we want to stop prior to hitting the max, and not rely on the wrap arround at all. So just cap at num_test_devs + 1, prior to assigning a new device. Link: http://lkml.kernel.org/r/20180224030046.24238-1-mcgrof@kernel.org Fixes: d9c6a72d6fa2 ("kmod: add test driver to stress test the module loader") Reported-by: Dan Carpenter Signed-off-by: Luis R. Rodriguez Acked-by: Kees Cook Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- lib/test_kmod.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/test_kmod.c b/lib/test_kmod.c index e372b97eee13..0e5b7a61460b 100644 --- a/lib/test_kmod.c +++ b/lib/test_kmod.c @@ -1141,7 +1141,7 @@ static struct kmod_test_device *register_test_dev_kmod(void) mutex_lock(®_dev_mutex); /* int should suffice for number of devices, test for wrap */ - if (unlikely(num_test_devs + 1) < 0) { + if (num_test_devs + 1 == INT_MAX) { pr_err("reached limit of number of test devices\n"); goto out; } -- cgit v1.2.3-58-ga151 From 8df3aaaf9b5f8bdfc4036695fa22f35b45b4d92f Mon Sep 17 00:00:00 2001 From: Joern Engel Date: Tue, 13 Mar 2018 11:36:49 -0700 Subject: btree: avoid variable-length allocations geo->keylen cannot be larger than 4. So we might as well make fixed-size allocations. Given the one remaining user, geo->keylen cannot even be larger than 1. Logfs used to have 64bit and 128bit keys, tcm_qla2xxx only has 32bit keys. But let's not break the code if we don't have to. Signed-off-by: Joern Engel Signed-off-by: Linus Torvalds --- lib/btree.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'lib') diff --git a/lib/btree.c b/lib/btree.c index f93a945274af..590facba2c50 100644 --- a/lib/btree.c +++ b/lib/btree.c @@ -3,7 +3,7 @@ * * As should be obvious for Linux kernel code, license is GPLv2 * - * Copyright (c) 2007-2008 Joern Engel + * Copyright (c) 2007-2008 Joern Engel * Bits and pieces stolen from Peter Zijlstra's code, which is * Copyright 2007, Red Hat Inc. Peter Zijlstra * GPLv2 @@ -76,6 +76,8 @@ struct btree_geo btree_geo128 = { }; EXPORT_SYMBOL_GPL(btree_geo128); +#define MAX_KEYLEN (2 * LONG_PER_U64) + static struct kmem_cache *btree_cachep; void *btree_alloc(gfp_t gfp_mask, void *pool_data) @@ -313,7 +315,7 @@ void *btree_get_prev(struct btree_head *head, struct btree_geo *geo, { int i, height; unsigned long *node, *oldnode; - unsigned long *retry_key = NULL, key[geo->keylen]; + unsigned long *retry_key = NULL, key[MAX_KEYLEN]; if (keyzero(geo, __key)) return NULL; @@ -639,8 +641,8 @@ EXPORT_SYMBOL_GPL(btree_remove); int btree_merge(struct btree_head *target, struct btree_head *victim, struct btree_geo *geo, gfp_t gfp) { - unsigned long key[geo->keylen]; - unsigned long dup[geo->keylen]; + unsigned long key[MAX_KEYLEN]; + unsigned long dup[MAX_KEYLEN]; void *val; int err; -- cgit v1.2.3-58-ga151