From 1a57b1a3e11086a4f183b245754b213b1d9b2d40 Mon Sep 17 00:00:00 2001 From: Zhen Lei Date: Wed, 18 Nov 2020 15:35:17 +0800 Subject: ACPI/nfit: avoid accessing uninitialized memory in acpi_nfit_ctl() The ACPI_ALLOCATE() does not zero the "buf", so when the condition "integer->type != ACPI_TYPE_INTEGER" in int_to_buf() is met, the result is unpredictable in acpi_nfit_ctl(). Signed-off-by: Zhen Lei Reviewed-by: Dan Williams Link: https://lore.kernel.org/r/20201118073517.1884-1-thunder.leizhen@huawei.com Signed-off-by: Dan Williams --- drivers/acpi/nfit/core.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'drivers') diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c index 442608220b5c..cda7b6c52504 100644 --- a/drivers/acpi/nfit/core.c +++ b/drivers/acpi/nfit/core.c @@ -282,18 +282,19 @@ err: static union acpi_object *int_to_buf(union acpi_object *integer) { - union acpi_object *buf = ACPI_ALLOCATE(sizeof(*buf) + 4); + union acpi_object *buf = NULL; void *dst = NULL; - if (!buf) - goto err; - if (integer->type != ACPI_TYPE_INTEGER) { WARN_ONCE(1, "BIOS bug, unexpected element type: %d\n", integer->type); goto err; } + buf = ACPI_ALLOCATE(sizeof(*buf) + 4); + if (!buf) + goto err; + dst = buf + 1; buf->type = ACPI_TYPE_BUFFER; buf->buffer.length = 4; -- cgit v1.2.3-59-g8ed1b From 2dd2a1740ee19cd2636d247276cf27bfa434b0e2 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Fri, 20 Nov 2020 08:50:07 -0800 Subject: libnvdimm/namespace: Fix reaping of invalidated block-window-namespace labels A recent change to ndctl to attempt to reconfigure namespaces in place uncovered a label accounting problem in block-window-type namespaces. The ndctl "create.sh" test is able to trigger this signature: WARNING: CPU: 34 PID: 9167 at drivers/nvdimm/label.c:1100 __blk_label_update+0x9a3/0xbc0 [libnvdimm] [..] RIP: 0010:__blk_label_update+0x9a3/0xbc0 [libnvdimm] [..] Call Trace: uuid_store+0x21b/0x2f0 [libnvdimm] kernfs_fop_write+0xcf/0x1c0 vfs_write+0xcc/0x380 ksys_write+0x68/0xe0 When allocated capacity for a namespace is renamed (new UUID) the labels with the old UUID need to be deleted. The ndctl behavior to always destroy namespaces on reconfiguration hid this problem. The immediate impact of this bug is limited since block-window-type namespaces only seem to exist in the specification and not in any shipping products. However, the label handling code is being reused for other technologies like CXL region labels, so there is a benefit to making sure both vertical labels sets (block-window) and horizontal label sets (pmem) have a functional reference implementation in libnvdimm. Fixes: c4703ce11c23 ("libnvdimm/namespace: Fix label tracking error") Cc: Cc: Vishal Verma Cc: Dave Jiang Cc: Ira Weiny Signed-off-by: Dan Williams --- drivers/nvdimm/label.c | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'drivers') diff --git a/drivers/nvdimm/label.c b/drivers/nvdimm/label.c index 47a4828b8b31..6f2be7a34598 100644 --- a/drivers/nvdimm/label.c +++ b/drivers/nvdimm/label.c @@ -980,6 +980,15 @@ static int __blk_label_update(struct nd_region *nd_region, } } + /* release slots associated with any invalidated UUIDs */ + mutex_lock(&nd_mapping->lock); + list_for_each_entry_safe(label_ent, e, &nd_mapping->labels, list) + if (test_and_clear_bit(ND_LABEL_REAP, &label_ent->flags)) { + reap_victim(nd_mapping, label_ent); + list_move(&label_ent->list, &list); + } + mutex_unlock(&nd_mapping->lock); + /* * Find the resource associated with the first label in the set * per the v1.2 namespace specification. -- cgit v1.2.3-59-g8ed1b From 9a7e3d7f056831a6193d6d737fb7a26dfdceb04b Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Mon, 23 Nov 2020 17:43:53 -0800 Subject: ACPI: NFIT: Fix input validation of bus-family Dan reports that smatch thinks userspace can craft an out-of-bound bus family number. However, nd_cmd_clear_to_send() blocks all non-zero values of bus-family since only the kernel can initiate these commands. However, in the speculation path, family is a user controlled array index value so mask it for speculation safety. Also, since the nd_cmd_clear_to_send() safety is non-obvious and possibly may change in the future include input validation as if userspace could get past the nd_cmd_clear_to_send() gatekeeper. Link: http://lore.kernel.org/r/20201111113000.GA1237157@mwanda Reported-by: Dan Carpenter Fixes: 6450ddbd5d8e ("ACPI: NFIT: Define runtime firmware activation commands") Cc: Signed-off-by: Dan Williams --- drivers/acpi/nfit/core.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c index cda7b6c52504..b11b08a60684 100644 --- a/drivers/acpi/nfit/core.c +++ b/drivers/acpi/nfit/core.c @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -479,8 +480,11 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm, cmd_mask = nd_desc->cmd_mask; if (cmd == ND_CMD_CALL && call_pkg->nd_family) { family = call_pkg->nd_family; - if (!test_bit(family, &nd_desc->bus_family_mask)) + if (family > NVDIMM_BUS_FAMILY_MAX || + !test_bit(family, &nd_desc->bus_family_mask)) return -EINVAL; + family = array_index_nospec(family, + NVDIMM_BUS_FAMILY_MAX + 1); dsm_mask = acpi_desc->family_dsm_mask[family]; guid = to_nfit_bus_uuid(family); } else { -- cgit v1.2.3-59-g8ed1b From 5ae96d779ff3eeb2977919ff311a6c8849943c2d Mon Sep 17 00:00:00 2001 From: Enrico Weigelt Date: Tue, 15 Dec 2020 17:35:31 +0100 Subject: libnvdimm: Cleanup include of badblocks.h * drivers/nvdimm/core.c doesn't use anything from badblocks.h on its own, thus including it isn't needed. There's indeed indirect use, via funcs in nd.h, but this one already includes badblocks.h. * drivers/nvdimm/claim.c calls stuff from badblocks.h and therefore should include it on its own (instead of relying any other header doing that) * drivers/nvdimm/btt.h doesn't really need anything from badblocks.h and can easily live with a forward declaration of struct badblocks (just having pointers to it, but not dereferencing it anywhere) Signed-off-by: Enrico Weigelt Link: https://lore.kernel.org/r/20201215163531.21446-1-info@metux.net Signed-off-by: Dan Williams --- drivers/nvdimm/btt.h | 3 ++- drivers/nvdimm/claim.c | 1 + drivers/nvdimm/core.c | 1 - 3 files changed, 3 insertions(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/nvdimm/btt.h b/drivers/nvdimm/btt.h index 2e258bee7db2..aa53e0b769bd 100644 --- a/drivers/nvdimm/btt.h +++ b/drivers/nvdimm/btt.h @@ -7,7 +7,6 @@ #ifndef _LINUX_BTT_H #define _LINUX_BTT_H -#include #include #define BTT_SIG_LEN 16 @@ -197,6 +196,8 @@ struct arena_info { int log_index[2]; }; +struct badblocks; + /** * struct btt - handle for a BTT instance * @btt_disk: Pointer to the gendisk for BTT device diff --git a/drivers/nvdimm/claim.c b/drivers/nvdimm/claim.c index 5a7c80053c62..030dbde6b088 100644 --- a/drivers/nvdimm/claim.c +++ b/drivers/nvdimm/claim.c @@ -4,6 +4,7 @@ */ #include #include +#include #include "nd-core.h" #include "pmem.h" #include "pfn.h" diff --git a/drivers/nvdimm/core.c b/drivers/nvdimm/core.c index c21ba0602029..7de592d7eff4 100644 --- a/drivers/nvdimm/core.c +++ b/drivers/nvdimm/core.c @@ -3,7 +3,6 @@ * Copyright(c) 2013-2015 Intel Corporation. All rights reserved. */ #include -#include #include #include #include -- cgit v1.2.3-59-g8ed1b From 4e6a7b3bbd5a6f9e6f0c5c3ad976ed116c7ade79 Mon Sep 17 00:00:00 2001 From: Zheng Yongjun Date: Mon, 14 Dec 2020 21:45:06 +0800 Subject: device-dax/pmem: Convert comma to semicolon Replace a comma between expression statements by a semicolon. Signed-off-by: Zheng Yongjun Link: https://lore.kernel.org/r/20201214134506.4831-1-zhengyongjun3@huawei.com Signed-off-by: Dan Williams --- drivers/dax/pmem/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/dax/pmem/core.c b/drivers/dax/pmem/core.c index 62b26bfceab1..062e8bc14223 100644 --- a/drivers/dax/pmem/core.c +++ b/drivers/dax/pmem/core.c @@ -52,7 +52,7 @@ struct dev_dax *__dax_pmem_probe(struct device *dev, enum dev_dax_subsys subsys) /* adjust the dax_region range to the start of data */ range = pgmap.range; - range.start += offset, + range.start += offset; dax_region = alloc_dax_region(dev, region_id, &range, nd_region->target_node, le32_to_cpu(pfn_sb->align), IORESOURCE_DAX_STATIC); -- cgit v1.2.3-59-g8ed1b From 1aa574312518ef1d60d2dc62d58f7021db3b163a Mon Sep 17 00:00:00 2001 From: Wang Hai Date: Tue, 1 Dec 2020 21:59:29 +0800 Subject: device-dax/core: Fix memory leak when rmmod dax.ko When I repeatedly modprobe and rmmod dax.ko, kmemleak report a memory leak as follows: unreferenced object 0xffff9a5588c05088 (size 8): comm "modprobe", pid 261, jiffies 4294693644 (age 42.063s) ... backtrace: [<00000000e007ced0>] kstrdup+0x35/0x70 [<000000002ae73897>] kstrdup_const+0x3d/0x50 [<000000002b00c9c3>] kvasprintf_const+0xbc/0xf0 [<000000008023282f>] kobject_set_name_vargs+0x3b/0xd0 [<00000000d2cbaa4e>] kobject_set_name+0x62/0x90 [<00000000202e7a22>] bus_register+0x7f/0x2b0 [<000000000b77792c>] 0xffffffffc02840f7 [<000000002d5be5ac>] 0xffffffffc02840b4 [<00000000dcafb7cd>] do_one_initcall+0x58/0x240 [<00000000049fe480>] do_init_module+0x56/0x1e2 [<0000000022671491>] load_module+0x2517/0x2840 [<000000001a2201cb>] __do_sys_finit_module+0x9c/0xe0 [<000000003eb304e7>] do_syscall_64+0x33/0x40 [<0000000051c5fd06>] entry_SYSCALL_64_after_hwframe+0x44/0xa9 When rmmod dax is executed, dax_bus_exit() is missing. This patch can fix this bug. Fixes: 9567da0b408a ("device-dax: Introduce bus + driver model") Cc: Reported-by: Hulk Robot Signed-off-by: Wang Hai Link: https://lore.kernel.org/r/20201201135929.66530-1-wanghai38@huawei.com Signed-off-by: Dan Williams --- drivers/dax/super.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers') diff --git a/drivers/dax/super.c b/drivers/dax/super.c index edc279be3e59..cadbd0a1a1ef 100644 --- a/drivers/dax/super.c +++ b/drivers/dax/super.c @@ -752,6 +752,7 @@ err_chrdev: static void __exit dax_core_exit(void) { + dax_bus_exit(); unregister_chrdev_region(dax_devt, MINORMASK+1); ida_destroy(&dax_minor_ida); dax_fs_exit(); -- cgit v1.2.3-59-g8ed1b From 4c46764733c85b82c07e9559b39da4d00a7dd659 Mon Sep 17 00:00:00 2001 From: Zhang Qilong Date: Sat, 5 Dec 2020 19:50:56 +0800 Subject: libnvdimm/label: Return -ENXIO for no slot in __blk_label_update Forget to set error code when nd_label_alloc_slot failed, and we add it to avoid overwritten error code. Fixes: 0ba1c634892b ("libnvdimm: write blk label set") Signed-off-by: Zhang Qilong Link: https://lore.kernel.org/r/20201205115056.2076523-1-zhangqilong3@huawei.com Signed-off-by: Dan Williams --- drivers/nvdimm/label.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/nvdimm/label.c b/drivers/nvdimm/label.c index 6f2be7a34598..9251441fd8a3 100644 --- a/drivers/nvdimm/label.c +++ b/drivers/nvdimm/label.c @@ -1008,8 +1008,10 @@ static int __blk_label_update(struct nd_region *nd_region, if (is_old_resource(res, old_res_list, old_num_resources)) continue; /* carry-over */ slot = nd_label_alloc_slot(ndd); - if (slot == UINT_MAX) + if (slot == UINT_MAX) { + rc = -ENXIO; goto abort; + } dev_dbg(ndd->dev, "allocated: %d\n", slot); nd_label = to_label(ndd, slot); -- cgit v1.2.3-59-g8ed1b From 8bcbe3132c66c07d03f64d5da80be753359f2e92 Mon Sep 17 00:00:00 2001 From: Zhen Lei Date: Fri, 20 Nov 2020 17:20:57 +0800 Subject: device-dax: delete a redundancy check in dev_dax_validate_align() After we have done the alignment check for the length of each range, the alignment check for dev_dax_size(dev_dax) is no longer needed, because it get the sum of the length of each range. Signed-off-by: Zhen Lei Link: https://lore.kernel.org/r/20201120092057.2144-1-thunder.leizhen@huawei.com Signed-off-by: Dan Williams --- drivers/dax/bus.c | 7 ------- 1 file changed, 7 deletions(-) (limited to 'drivers') diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c index 27513d311242..9761cb40d4bb 100644 --- a/drivers/dax/bus.c +++ b/drivers/dax/bus.c @@ -1114,16 +1114,9 @@ static ssize_t align_show(struct device *dev, static ssize_t dev_dax_validate_align(struct dev_dax *dev_dax) { - resource_size_t dev_size = dev_dax_size(dev_dax); struct device *dev = &dev_dax->dev; int i; - if (dev_size > 0 && !alloc_is_aligned(dev_dax, dev_size)) { - dev_dbg(dev, "%s: align %u invalid for size %pa\n", - __func__, dev_dax->align, &dev_size); - return -EINVAL; - } - for (i = 0; i < dev_dax->nr_range; i++) { size_t len = range_len(&dev_dax->ranges[i].range); -- cgit v1.2.3-59-g8ed1b From 6268d7da4d192af339f4d688942b9ccb45a65e04 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Fri, 18 Dec 2020 18:41:41 -0800 Subject: device-dax: Fix range release There are multiple locations that open-code the release of the last range in a device-dax instance. Consolidate this into a new dev_dax_trim_range() helper. This also addresses a kmemleak report: # cat /sys/kernel/debug/kmemleak [..] unreferenced object 0xffff976bd46f6240 (size 64): comm "ndctl", pid 23556, jiffies 4299514316 (age 5406.733s) hex dump (first 32 bytes): 00 00 00 00 00 00 00 00 00 00 20 c3 37 00 00 00 .......... .7... ff ff ff 7f 38 00 00 00 00 00 00 00 00 00 00 00 ....8........... backtrace: [<00000000064003cf>] __kmalloc_track_caller+0x136/0x379 [<00000000d85e3c52>] krealloc+0x67/0x92 [<00000000d7d3ba8a>] __alloc_dev_dax_range+0x73/0x25c [<0000000027d58626>] devm_create_dev_dax+0x27d/0x416 [<00000000434abd43>] __dax_pmem_probe+0x1c9/0x1000 [dax_pmem_core] [<0000000083726c1c>] dax_pmem_probe+0x10/0x1f [dax_pmem] [<00000000b5f2319c>] nvdimm_bus_probe+0x9d/0x340 [libnvdimm] [<00000000c055e544>] really_probe+0x230/0x48d [<000000006cabd38e>] driver_probe_device+0x122/0x13b [<0000000029c7b95a>] device_driver_attach+0x5b/0x60 [<0000000053e5659b>] bind_store+0xb7/0xc3 [<00000000d3bdaadc>] drv_attr_store+0x27/0x31 [<00000000949069c5>] sysfs_kf_write+0x4a/0x57 [<000000004a8b5adf>] kernfs_fop_write+0x150/0x1e5 [<00000000bded60f0>] __vfs_write+0x1b/0x34 [<00000000b92900f0>] vfs_write+0xd8/0x1d1 Reported-by: Jane Chu Cc: Zhen Lei Link: https://lore.kernel.org/r/160834570161.1791850.14911670304441510419.stgit@dwillia2-desk3.amr.corp.intel.com Signed-off-by: Dan Williams --- drivers/dax/bus.c | 44 +++++++++++++++++++++----------------------- 1 file changed, 21 insertions(+), 23 deletions(-) (limited to 'drivers') diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c index 9761cb40d4bb..720cd140209f 100644 --- a/drivers/dax/bus.c +++ b/drivers/dax/bus.c @@ -367,19 +367,28 @@ void kill_dev_dax(struct dev_dax *dev_dax) } EXPORT_SYMBOL_GPL(kill_dev_dax); -static void free_dev_dax_ranges(struct dev_dax *dev_dax) +static void trim_dev_dax_range(struct dev_dax *dev_dax) { + int i = dev_dax->nr_range - 1; + struct range *range = &dev_dax->ranges[i].range; struct dax_region *dax_region = dev_dax->region; - int i; device_lock_assert(dax_region->dev); - for (i = 0; i < dev_dax->nr_range; i++) { - struct range *range = &dev_dax->ranges[i].range; - - __release_region(&dax_region->res, range->start, - range_len(range)); + dev_dbg(&dev_dax->dev, "delete range[%d]: %#llx:%#llx\n", i, + (unsigned long long)range->start, + (unsigned long long)range->end); + + __release_region(&dax_region->res, range->start, range_len(range)); + if (--dev_dax->nr_range == 0) { + kfree(dev_dax->ranges); + dev_dax->ranges = NULL; } - dev_dax->nr_range = 0; +} + +static void free_dev_dax_ranges(struct dev_dax *dev_dax) +{ + while (dev_dax->nr_range) + trim_dev_dax_range(dev_dax); } static void unregister_dev_dax(void *dev) @@ -804,15 +813,10 @@ static int alloc_dev_dax_range(struct dev_dax *dev_dax, u64 start, return 0; rc = devm_register_dax_mapping(dev_dax, dev_dax->nr_range - 1); - if (rc) { - dev_dbg(dev, "delete range[%d]: %pa:%pa\n", dev_dax->nr_range - 1, - &alloc->start, &alloc->end); - dev_dax->nr_range--; - __release_region(res, alloc->start, resource_size(alloc)); - return rc; - } + if (rc) + trim_dev_dax_range(dev_dax); - return 0; + return rc; } static int adjust_dev_dax_range(struct dev_dax *dev_dax, struct resource *res, resource_size_t size) @@ -885,12 +889,7 @@ static int dev_dax_shrink(struct dev_dax *dev_dax, resource_size_t size) if (shrink >= range_len(range)) { devm_release_action(dax_region->dev, unregister_dax_mapping, &mapping->dev); - __release_region(&dax_region->res, range->start, - range_len(range)); - dev_dax->nr_range--; - dev_dbg(dev, "delete range[%d]: %#llx:%#llx\n", i, - (unsigned long long) range->start, - (unsigned long long) range->end); + trim_dev_dax_range(dev_dax); to_shrink -= shrink; if (!to_shrink) break; @@ -1267,7 +1266,6 @@ static void dev_dax_release(struct device *dev) put_dax(dax_dev); free_dev_dax_id(dev_dax); dax_region_put(dax_region); - kfree(dev_dax->ranges); kfree(dev_dax->pgmap); kfree(dev_dax); } -- cgit v1.2.3-59-g8ed1b From ff8da37d3d8d438ded5a4841d979899269b94d0d Mon Sep 17 00:00:00 2001 From: Zhen Lei Date: Sat, 19 Dec 2020 16:18:40 +0800 Subject: device-dax: Avoid an unnecessary check in alloc_dev_dax_range() Swap the calling sequence of krealloc() and __request_region(), call the latter first. In this way, the value of dev_dax->nr_range does not need to be considered when __request_region() failed. Signed-off-by: Zhen Lei Link: https://lore.kernel.org/r/20201219081840.1149-2-thunder.leizhen@huawei.com Signed-off-by: Dan Williams --- drivers/dax/bus.c | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) (limited to 'drivers') diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c index 720cd140209f..737b207c9e30 100644 --- a/drivers/dax/bus.c +++ b/drivers/dax/bus.c @@ -772,22 +772,14 @@ static int alloc_dev_dax_range(struct dev_dax *dev_dax, u64 start, return 0; } - ranges = krealloc(dev_dax->ranges, sizeof(*ranges) - * (dev_dax->nr_range + 1), GFP_KERNEL); - if (!ranges) + alloc = __request_region(res, start, size, dev_name(dev), 0); + if (!alloc) return -ENOMEM; - alloc = __request_region(res, start, size, dev_name(dev), 0); - if (!alloc) { - /* - * If this was an empty set of ranges nothing else - * will release @ranges, so do it now. - */ - if (!dev_dax->nr_range) { - kfree(ranges); - ranges = NULL; - } - dev_dax->ranges = ranges; + ranges = krealloc(dev_dax->ranges, sizeof(*ranges) + * (dev_dax->nr_range + 1), GFP_KERNEL); + if (!ranges) { + __release_region(res, alloc->start, resource_size(alloc)); return -ENOMEM; } -- cgit v1.2.3-59-g8ed1b