From e1dc20995cb9fa04b46e8f37113a7203c906d2bf Mon Sep 17 00:00:00 2001 From: John Garry Date: Mon, 21 Dec 2020 22:30:55 +0800 Subject: driver core: platform: Add extra error check in devm_platform_get_irqs_affinity() The current check of nvec < minvec for nvec returned from platform_irq_count() will not detect a negative error code in nvec. This is because minvec is unsigned, and, as such, nvec is promoted to unsigned in that check, which will make it a huge number (if it contained -EPROBE_DEFER). In practice, an error should not occur in nvec for the only in-tree user, but add a check anyway. Fixes: e15f2fa959f2 ("driver core: platform: Add devm_platform_get_irqs_affinity()") Reported-by: Dan Carpenter Signed-off-by: John Garry Signed-off-by: Marc Zyngier Link: https://lore.kernel.org/r/1608561055-231244-1-git-send-email-john.garry@huawei.com --- drivers/base/platform.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/base') diff --git a/drivers/base/platform.c b/drivers/base/platform.c index ea8add164b89..74c97b65048c 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -351,6 +351,8 @@ int devm_platform_get_irqs_affinity(struct platform_device *dev, return -ERANGE; nvec = platform_irq_count(dev); + if (nvec < 0) + return nvec; if (nvec < minvec) return -ENOSPC; -- cgit v1.2.3 From cffa4b2122f5f3e53cf3d529bbc74651f95856d5 Mon Sep 17 00:00:00 2001 From: Xiaolei Wang Date: Tue, 29 Dec 2020 18:50:46 +0800 Subject: regmap: debugfs: Fix a memory leak when calling regmap_attach_dev After initializing the regmap through syscon_regmap_lookup_by_compatible, then regmap_attach_dev to the device, because the debugfs_name has been allocated, there is no need to redistribute it again unreferenced object 0xd8399b80 (size 64): comm "swapper/0", pid 1, jiffies 4294937641 (age 278.590s) hex dump (first 32 bytes): 64 75 6d 6d 79 2d 69 6f 6d 75 78 63 2d 67 70 72 dummy-iomuxc-gpr 40 32 30 65 34 30 30 30 00 7f 52 5b d8 7e 42 69 @20e4000..R[.~Bi backtrace: [] kasprintf+0x2c/0x54 [<6ad3bbc2>] regmap_debugfs_init+0xdc/0x2fc [] __regmap_init+0xc38/0xd88 [<1f7e0609>] of_syscon_register+0x168/0x294 [<735e8766>] device_node_get_regmap+0x6c/0x98 [] imx6ul_init_machine+0x20/0x88 [<0456565b>] customize_machine+0x1c/0x30 [] do_one_initcall+0x80/0x3ac [<7e584867>] kernel_init_freeable+0x170/0x1f0 [<80074741>] kernel_init+0x8/0x120 [<285d6f28>] ret_from_fork+0x14/0x20 [<00000000>] 0x0 Fixes: 9b947a13e7f6 ("regmap: use debugfs even when no device") Signed-off-by: Xiaolei Wang Link: https://lore.kernel.org/r/20201229105046.41984-1-xiaolei.wang@windriver.com Signed-off-by: Mark Brown --- drivers/base/regmap/regmap-debugfs.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/regmap/regmap-debugfs.c b/drivers/base/regmap/regmap-debugfs.c index 8dfac7f3ed7a..bf03cd343be2 100644 --- a/drivers/base/regmap/regmap-debugfs.c +++ b/drivers/base/regmap/regmap-debugfs.c @@ -582,18 +582,25 @@ void regmap_debugfs_init(struct regmap *map) devname = dev_name(map->dev); if (name) { - map->debugfs_name = kasprintf(GFP_KERNEL, "%s-%s", + if (!map->debugfs_name) { + map->debugfs_name = kasprintf(GFP_KERNEL, "%s-%s", devname, name); + if (!map->debugfs_name) + return; + } name = map->debugfs_name; } else { name = devname; } if (!strcmp(name, "dummy")) { - kfree(map->debugfs_name); + if (!map->debugfs_name) + kfree(map->debugfs_name); map->debugfs_name = kasprintf(GFP_KERNEL, "dummy%d", dummy_index); + if (!map->debugfs_name) + return; name = map->debugfs_name; dummy_index++; } -- cgit v1.2.3 From f6bcb4c7f366905b66ce8ffca7190118244bb642 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Tue, 5 Jan 2021 14:42:29 +0300 Subject: regmap: debugfs: Fix a reversed if statement in regmap_debugfs_init() This code will leak "map->debugfs_name" because the if statement is reversed so it only frees NULL pointers instead of non-NULL. In fact the if statement is not required and should just be removed because kfree() accepts NULL pointers. Fixes: cffa4b2122f5 ("regmap: debugfs: Fix a memory leak when calling regmap_attach_dev") Signed-off-by: Dan Carpenter Link: https://lore.kernel.org/r/X/RQpfAwRdLg0GqQ@mwanda Signed-off-by: Mark Brown --- drivers/base/regmap/regmap-debugfs.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/regmap/regmap-debugfs.c b/drivers/base/regmap/regmap-debugfs.c index bf03cd343be2..ff2ee87987c7 100644 --- a/drivers/base/regmap/regmap-debugfs.c +++ b/drivers/base/regmap/regmap-debugfs.c @@ -594,9 +594,7 @@ void regmap_debugfs_init(struct regmap *map) } if (!strcmp(name, "dummy")) { - if (!map->debugfs_name) - kfree(map->debugfs_name); - + kfree(map->debugfs_name); map->debugfs_name = kasprintf(GFP_KERNEL, "dummy%d", dummy_index); if (!map->debugfs_name) -- cgit v1.2.3 From 47f4469970d8861bc06d2d4d45ac8200ff07c693 Mon Sep 17 00:00:00 2001 From: Bard Liao Date: Tue, 5 Jan 2021 17:11:45 +0800 Subject: Revert "device property: Keep secondary firmware node secondary by type" While commit d5dcce0c414f ("device property: Keep secondary firmware node secondary by type") describes everything correct in its commit message, the change it made does the opposite and original commit c15e1bdda436 ("device property: Fix the secondary firmware node handling in set_primary_fwnode()") was fully correct. Revert the former one here and improve documentation in the next patch. Fixes: d5dcce0c414f ("device property: Keep secondary firmware node secondary by type") Signed-off-by: Bard Liao Reviewed-by: Andy Shevchenko Reviewed-by: Heikki Krogerus Cc: 5.10+ # 5.10+ Signed-off-by: Rafael J. Wysocki --- drivers/base/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/base') diff --git a/drivers/base/core.c b/drivers/base/core.c index 25e08e5f40bd..51b9545a050b 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -4433,7 +4433,7 @@ void set_primary_fwnode(struct device *dev, struct fwnode_handle *fwnode) if (fwnode_is_primary(fn)) { dev->fwnode = fn->secondary; if (!(parent && fn == parent->fwnode)) - fn->secondary = ERR_PTR(-ENODEV); + fn->secondary = NULL; } else { dev->fwnode = NULL; } -- cgit v1.2.3 From 3f7bddaf5d5a83aa2eb1e6d72db221d3ec43c813 Mon Sep 17 00:00:00 2001 From: Bard Liao Date: Tue, 5 Jan 2021 17:11:46 +0800 Subject: device property: add description of fwnode cases There are only four valid fwnode cases which are - primary --> secondary --> -ENODEV - primary --> NULL - secondary --> -ENODEV - NULL dev->fwnode should be converted between the 4 cases above no matter how/when set_primary_fwnode() and set_secondary_fwnode() are called. Describe it in the code so people will keep it in mind. Signed-off-by: Bard Liao Reviewed-by: Andy Shevchenko Reviewed-by: Heikki Krogerus [ rjw: Comment edit ] Signed-off-by: Rafael J. Wysocki --- drivers/base/core.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'drivers/base') diff --git a/drivers/base/core.c b/drivers/base/core.c index 51b9545a050b..14f165816742 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -4414,6 +4414,12 @@ static inline bool fwnode_is_primary(struct fwnode_handle *fwnode) * * Set the device's firmware node pointer to @fwnode, but if a secondary * firmware node of the device is present, preserve it. + * + * Valid fwnode cases are: + * - primary --> secondary --> -ENODEV + * - primary --> NULL + * - secondary --> -ENODEV + * - NULL */ void set_primary_fwnode(struct device *dev, struct fwnode_handle *fwnode) { @@ -4432,6 +4438,7 @@ void set_primary_fwnode(struct device *dev, struct fwnode_handle *fwnode) } else { if (fwnode_is_primary(fn)) { dev->fwnode = fn->secondary; + /* Set fn->secondary = NULL, so fn remains the primary fwnode */ if (!(parent && fn == parent->fwnode)) fn->secondary = NULL; } else { -- cgit v1.2.3 From d0243bbd5dd3ebbd49dafa8b56bb911d971131d0 Mon Sep 17 00:00:00 2001 From: Meng Li Date: Tue, 5 Jan 2021 15:09:27 +0800 Subject: drivers core: Free dma_range_map when driver probe failed There will be memory leak if driver probe failed. Trace as below: backtrace: [<000000002415258f>] kmemleak_alloc+0x3c/0x50 [<00000000f447ebe4>] __kmalloc+0x208/0x530 [<0000000048bc7b3a>] of_dma_get_range+0xe4/0x1b0 [<0000000041e39065>] of_dma_configure_id+0x58/0x27c [<000000006356866a>] platform_dma_configure+0x2c/0x40 ...... [<000000000afcf9b5>] ret_from_fork+0x10/0x3c This issue is introduced by commit e0d072782c73("dma-mapping: introduce DMA range map, supplanting dma_pfn_offset "). It doesn't free dma_range_map when driver probe failed and cause above memory leak. So, add code to free it in error path. Fixes: e0d072782c73 ("dma-mapping: introduce DMA range map, supplanting dma_pfn_offset ") Cc: stable@vger.kernel.org Signed-off-by: Meng Li Link: https://lore.kernel.org/r/20210105070927.14968-1-Meng.Li@windriver.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/dd.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/base') diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 2f32f38a11ed..0b76b5423778 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -619,6 +619,8 @@ dev_groups_failed: else if (drv->remove) drv->remove(dev); probe_failed: + kfree(dev->dma_range_map); + dev->dma_range_map = NULL; if (dev->bus) blocking_notifier_call_chain(&dev->bus->p->bus_notifier, BUS_NOTIFY_DRIVER_NOT_BOUND, dev); -- cgit v1.2.3 From 29f7c54b253fc18bff9bf7e9f303b75deb285c7a Mon Sep 17 00:00:00 2001 From: John Garry Date: Mon, 21 Dec 2020 22:30:55 +0800 Subject: Driver core: platform: Add extra error check in devm_platform_get_irqs_affinity() The current check of nvec < minvec for nvec returned from platform_irq_count() will not detect a negative error code in nvec. This is because minvec is unsigned, and, as such, nvec is promoted to unsigned in that check, which will make it a huge number (if it contained -EPROBE_DEFER). In practice, an error should not occur in nvec for the only in-tree user, but add a check anyway. Fixes: e15f2fa959f2 ("driver core: platform: Add devm_platform_get_irqs_affinity()") Reported-by: Dan Carpenter Signed-off-by: John Garry Link: https://lore.kernel.org/r/1608561055-231244-1-git-send-email-john.garry@huawei.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/platform.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/base') diff --git a/drivers/base/platform.c b/drivers/base/platform.c index 95fd1549f87d..8456d8384ac8 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -366,6 +366,8 @@ int devm_platform_get_irqs_affinity(struct platform_device *dev, return -ERANGE; nvec = platform_irq_count(dev); + if (nvec < 0) + return nvec; if (nvec < minvec) return -ENOSPC; -- cgit v1.2.3 From 895bee270863588fe3d46dca86cd15d461f47a7a Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Tue, 12 Jan 2021 19:02:29 +0100 Subject: Revert "driver core: Reorder devices on successful probe" This reverts commit 5b6164d3465fcc13b5679c860c452963443172a7. Stephan reports problems with this commit, so revert it for now. Fixes: 5b6164d3465f ("driver core: Reorder devices on successful probe") Link: https://lore.kernel.org/r/X/ycQpu7NIGI969v@gerhold.net Reported-by: Stephan Gerhold Cc: Jonathan Hunter Cc: Rafael. J. Wysocki Cc: Thierry Reding Signed-off-by: Greg Kroah-Hartman --- drivers/base/dd.c | 7 ------- 1 file changed, 7 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 0b76b5423778..9179825ff646 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -370,13 +370,6 @@ static void driver_bound(struct device *dev) device_pm_check_callbacks(dev); - /* - * Reorder successfully probed devices to the end of the device list. - * This ensures that suspend/resume order matches probe order, which - * is usually what drivers rely on. - */ - device_pm_move_to_tail(dev); - /* * Make sure the device is no longer in one of the deferred lists and * kick off retrying all pending devices -- cgit v1.2.3 From 3d1cf435e201d1fd63e4346b141881aed086effd Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 15 Jan 2021 19:30:51 +0100 Subject: driver core: Extend device_is_dependent() If the device passed as the target (second argument) to device_is_dependent() is not completely registered (that is, it has been initialized, but not added yet), but the parent pointer of it is set, it may be missing from the list of the parent's children and device_for_each_child() called by device_is_dependent() cannot be relied on to catch that dependency. For this reason, modify device_is_dependent() to check the ancestors of the target device by following its parent pointer in addition to the device_for_each_child() walk. Fixes: 9ed9895370ae ("driver core: Functional dependencies tracking support") Reported-by: Stephan Gerhold Tested-by: Stephan Gerhold Reviewed-by: Saravana Kannan Signed-off-by: Rafael J. Wysocki Link: https://lore.kernel.org/r/17705994.d592GUb2YH@kreacher Cc: stable Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) (limited to 'drivers/base') diff --git a/drivers/base/core.c b/drivers/base/core.c index 25e08e5f40bd..3819fd012e27 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -208,6 +208,16 @@ int device_links_read_lock_held(void) #endif #endif /* !CONFIG_SRCU */ +static bool device_is_ancestor(struct device *dev, struct device *target) +{ + while (target->parent) { + target = target->parent; + if (dev == target) + return true; + } + return false; +} + /** * device_is_dependent - Check if one device depends on another one * @dev: Device to check dependencies for. @@ -221,7 +231,12 @@ int device_is_dependent(struct device *dev, void *target) struct device_link *link; int ret; - if (dev == target) + /* + * The "ancestors" check is needed to catch the case when the target + * device has not been completely initialized yet and it is still + * missing from the list of children of its parent device. + */ + if (dev == target || device_is_ancestor(dev, target)) return 1; ret = device_for_each_child(dev, target, device_is_dependent); -- cgit v1.2.3 From e020ff611ba9be54e959e6b548038f8a020da1c9 Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Sun, 10 Jan 2021 09:54:07 -0800 Subject: driver core: Fix device link device name collision The device link device's name was of the form: -- This can cause name collision as reported here [1] as device names are not globally unique. Since device names have to be unique within the bus/class, add the bus/class name as a prefix to the device names used to construct the device link device name. So the devuce link device's name will be of the form: :--: [1] - https://lore.kernel.org/lkml/20201229033440.32142-1-michael@walle.cc/ Fixes: 287905e68dd2 ("driver core: Expose device link details in sysfs") Cc: stable@vger.kernel.org Reported-by: Michael Walle Tested-by: Michael Walle Signed-off-by: Saravana Kannan Link: https://lore.kernel.org/r/20210110175408.1465657-1-saravanak@google.com Signed-off-by: Greg Kroah-Hartman --- Documentation/ABI/testing/sysfs-class-devlink | 4 ++-- Documentation/ABI/testing/sysfs-devices-consumer | 5 +++-- Documentation/ABI/testing/sysfs-devices-supplier | 5 +++-- drivers/base/core.c | 27 +++++++++++++----------- include/linux/device.h | 12 +++++++++++ 5 files changed, 35 insertions(+), 18 deletions(-) (limited to 'drivers/base') diff --git a/Documentation/ABI/testing/sysfs-class-devlink b/Documentation/ABI/testing/sysfs-class-devlink index b662f747c83e..8a21ce515f61 100644 --- a/Documentation/ABI/testing/sysfs-class-devlink +++ b/Documentation/ABI/testing/sysfs-class-devlink @@ -5,8 +5,8 @@ Description: Provide a place in sysfs for the device link objects in the kernel at any given time. The name of a device link directory, denoted as ... above, is of the form -- - where is the supplier device name and is - the consumer device name. + where is the supplier bus:device name and + is the consumer bus:device name. What: /sys/class/devlink/.../auto_remove_on Date: May 2020 diff --git a/Documentation/ABI/testing/sysfs-devices-consumer b/Documentation/ABI/testing/sysfs-devices-consumer index 1f06d74d1c3c..0809fda092e6 100644 --- a/Documentation/ABI/testing/sysfs-devices-consumer +++ b/Documentation/ABI/testing/sysfs-devices-consumer @@ -4,5 +4,6 @@ Contact: Saravana Kannan Description: The /sys/devices/.../consumer: are symlinks to device links where this device is the supplier. denotes the - name of the consumer in that device link. There can be zero or - more of these symlinks for a given device. + name of the consumer in that device link and is of the form + bus:device name. There can be zero or more of these symlinks + for a given device. diff --git a/Documentation/ABI/testing/sysfs-devices-supplier b/Documentation/ABI/testing/sysfs-devices-supplier index a919e0db5e90..207f5972e98d 100644 --- a/Documentation/ABI/testing/sysfs-devices-supplier +++ b/Documentation/ABI/testing/sysfs-devices-supplier @@ -4,5 +4,6 @@ Contact: Saravana Kannan Description: The /sys/devices/.../supplier: are symlinks to device links where this device is the consumer. denotes the - name of the supplier in that device link. There can be zero or - more of these symlinks for a given device. + name of the supplier in that device link and is of the form + bus:device name. There can be zero or more of these symlinks + for a given device. diff --git a/drivers/base/core.c b/drivers/base/core.c index 3819fd012e27..78f6db169d47 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -471,7 +471,9 @@ static int devlink_add_symlinks(struct device *dev, struct device *con = link->consumer; char *buf; - len = max(strlen(dev_name(sup)), strlen(dev_name(con))); + len = max(strlen(dev_bus_name(sup)) + strlen(dev_name(sup)), + strlen(dev_bus_name(con)) + strlen(dev_name(con))); + len += strlen(":"); len += strlen("supplier:") + 1; buf = kzalloc(len, GFP_KERNEL); if (!buf) @@ -485,12 +487,12 @@ static int devlink_add_symlinks(struct device *dev, if (ret) goto err_con; - snprintf(buf, len, "consumer:%s", dev_name(con)); + snprintf(buf, len, "consumer:%s:%s", dev_bus_name(con), dev_name(con)); ret = sysfs_create_link(&sup->kobj, &link->link_dev.kobj, buf); if (ret) goto err_con_dev; - snprintf(buf, len, "supplier:%s", dev_name(sup)); + snprintf(buf, len, "supplier:%s:%s", dev_bus_name(sup), dev_name(sup)); ret = sysfs_create_link(&con->kobj, &link->link_dev.kobj, buf); if (ret) goto err_sup_dev; @@ -498,7 +500,7 @@ static int devlink_add_symlinks(struct device *dev, goto out; err_sup_dev: - snprintf(buf, len, "consumer:%s", dev_name(con)); + snprintf(buf, len, "consumer:%s:%s", dev_bus_name(con), dev_name(con)); sysfs_remove_link(&sup->kobj, buf); err_con_dev: sysfs_remove_link(&link->link_dev.kobj, "consumer"); @@ -521,7 +523,9 @@ static void devlink_remove_symlinks(struct device *dev, sysfs_remove_link(&link->link_dev.kobj, "consumer"); sysfs_remove_link(&link->link_dev.kobj, "supplier"); - len = max(strlen(dev_name(sup)), strlen(dev_name(con))); + len = max(strlen(dev_bus_name(sup)) + strlen(dev_name(sup)), + strlen(dev_bus_name(con)) + strlen(dev_name(con))); + len += strlen(":"); len += strlen("supplier:") + 1; buf = kzalloc(len, GFP_KERNEL); if (!buf) { @@ -529,9 +533,9 @@ static void devlink_remove_symlinks(struct device *dev, return; } - snprintf(buf, len, "supplier:%s", dev_name(sup)); + snprintf(buf, len, "supplier:%s:%s", dev_bus_name(sup), dev_name(sup)); sysfs_remove_link(&con->kobj, buf); - snprintf(buf, len, "consumer:%s", dev_name(con)); + snprintf(buf, len, "consumer:%s:%s", dev_bus_name(con), dev_name(con)); sysfs_remove_link(&sup->kobj, buf); kfree(buf); } @@ -752,8 +756,9 @@ struct device_link *device_link_add(struct device *consumer, link->link_dev.class = &devlink_class; device_set_pm_not_required(&link->link_dev); - dev_set_name(&link->link_dev, "%s--%s", - dev_name(supplier), dev_name(consumer)); + dev_set_name(&link->link_dev, "%s:%s--%s:%s", + dev_bus_name(supplier), dev_name(supplier), + dev_bus_name(consumer), dev_name(consumer)); if (device_register(&link->link_dev)) { put_device(consumer); put_device(supplier); @@ -1823,9 +1828,7 @@ const char *dev_driver_string(const struct device *dev) * never change once they are set, so they don't need special care. */ drv = READ_ONCE(dev->driver); - return drv ? drv->name : - (dev->bus ? dev->bus->name : - (dev->class ? dev->class->name : "")); + return drv ? drv->name : dev_bus_name(dev); } EXPORT_SYMBOL(dev_driver_string); diff --git a/include/linux/device.h b/include/linux/device.h index 89bb8b84173e..1779f90eeb4c 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -609,6 +609,18 @@ static inline const char *dev_name(const struct device *dev) return kobject_name(&dev->kobj); } +/** + * dev_bus_name - Return a device's bus/class name, if at all possible + * @dev: struct device to get the bus/class name of + * + * Will return the name of the bus/class the device is attached to. If it is + * not attached to a bus/class, an empty string will be returned. + */ +static inline const char *dev_bus_name(const struct device *dev) +{ + return dev->bus ? dev->bus->name : (dev->class ? dev->class->name : ""); +} + __printf(2, 3) int dev_set_name(struct device *dev, const char *name, ...); #ifdef CONFIG_NUMA -- cgit v1.2.3