From 07b7b883be5ba0b4bd9ebf8d72c236ef36ae2676 Mon Sep 17 00:00:00 2001 From: Wolfram Sang Date: Thu, 18 Aug 2022 22:59:56 +0200 Subject: driver_core: move from strlcpy with unused retval to strscpy Follow the advice of the below link and prefer 'strscpy' in this subsystem. Conversion is 1:1 because the return value is not used. Generated by a coccinelle script. Link: https://lore.kernel.org/r/CAHk-=wgfRnXz0W3D37d01q3JFkr_i_uTL=V6A6G1oUZcprmknw@mail.gmail.com/ Signed-off-by: Wolfram Sang Link: https://lore.kernel.org/r/20220818205956.6528-1-wsa+renesas@sang-engineering.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/dd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/base') diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 70f79fc71539..428265a04321 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -806,7 +806,7 @@ static int __init save_async_options(char *buf) if (strlen(buf) >= ASYNC_DRV_NAMES_MAX_LEN) pr_warn("Too long list of driver names for 'driver_async_probe'!\n"); - strlcpy(async_probe_drv_names, buf, ASYNC_DRV_NAMES_MAX_LEN); + strscpy(async_probe_drv_names, buf, ASYNC_DRV_NAMES_MAX_LEN); async_probe_default = parse_option_str(async_probe_drv_names, "*"); return 1; -- cgit v1.2.3 From e9628e015fe205f10766f031f17e217f85650570 Mon Sep 17 00:00:00 2001 From: Yang Yingliang Date: Mon, 22 Aug 2022 14:19:22 +0800 Subject: class: use IS_ERR_OR_NULL() helper in class_unregister() Use IS_ERR_OR_NULL() helper in class_unregister() to simplify code. Signed-off-by: Yang Yingliang Link: https://lore.kernel.org/r/20220822061922.3884113-1-yangyingliang@huawei.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/class.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/base') diff --git a/drivers/base/class.c b/drivers/base/class.c index 8feb85e186e3..64f7b9a0970f 100644 --- a/drivers/base/class.c +++ b/drivers/base/class.c @@ -260,7 +260,7 @@ EXPORT_SYMBOL_GPL(__class_create); */ void class_destroy(struct class *cls) { - if ((cls == NULL) || (IS_ERR(cls))) + if (IS_ERR_OR_NULL(cls)) return; class_unregister(cls); -- cgit v1.2.3 From 6bb7ea3afd3eaae95eda6ef5589c3087f2cd70d8 Mon Sep 17 00:00:00 2001 From: Brian Norris Date: Wed, 24 Aug 2022 16:52:16 -0700 Subject: drivers: base: Print error code on synthetic uevent failure If we're going to log the failure, we might as well log the return code too. Signed-off-by: Brian Norris Link: https://lore.kernel.org/r/20220824165213.1.Ifdb98af3d0c23708a11d8d5ae5697bdb7e96a3cc@changeid Signed-off-by: Greg Kroah-Hartman --- 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 753e7cca0f40..fd0410329046 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -2509,7 +2509,7 @@ static ssize_t uevent_store(struct device *dev, struct device_attribute *attr, rc = kobject_synth_uevent(&dev->kobj, buf, count); if (rc) { - dev_err(dev, "uevent: failed to send synthetic uevent\n"); + dev_err(dev, "uevent: failed to send synthetic uevent: %d\n", rc); return rc; } -- cgit v1.2.3 From 61742a7cd5b194d2cc52d78de8ec6967634a4cd6 Mon Sep 17 00:00:00 2001 From: Christophe JAILLET Date: Wed, 20 Jul 2022 23:26:36 +0200 Subject: devres: Slightly optimize alloc_dr() If the gfp flag used for the memory allocation already has __GFP_ZERO, then there is no need to explicitly clear the "struct devres_node". It is already zeroed. This saves a few cycles when using devm_zalloc() and co. In the case of devres_alloc() (which calls __devres_alloc_node()), the compiler could remove the test and the memset() because it should be able to see that the __GFP_ZERO flag is set. So this would make the code both faster and smaller. Signed-off-by: Christophe JAILLET Link: https://lore.kernel.org/r/d255bd871484e63cdd628e819f929e2df59afb02.1658352383.git.christophe.jaillet@wanadoo.fr Signed-off-by: Greg Kroah-Hartman --- drivers/base/devres.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers/base') diff --git a/drivers/base/devres.c b/drivers/base/devres.c index 864d0b3f566e..4ab2b50ee38f 100644 --- a/drivers/base/devres.c +++ b/drivers/base/devres.c @@ -117,7 +117,9 @@ static __always_inline struct devres * alloc_dr(dr_release_t release, if (unlikely(!dr)) return NULL; - memset(dr, 0, offsetof(struct devres, data)); + /* No need to clear memory twice */ + if (!(gfp & __GFP_ZERO)) + memset(dr, 0, offsetof(struct devres, data)); INIT_LIST_HEAD(&dr->node.entry); dr->node.release = release; -- cgit v1.2.3 From d11b1e908e9a1301e43cefc37fc17dd2b1257b77 Mon Sep 17 00:00:00 2001 From: Gaosheng Cui Date: Fri, 9 Sep 2022 14:33:37 +0800 Subject: driver core: remove make_class_name declaration make_class_name has been removed since commit 39aba963d937 ("driver core: remove CONFIG_SYSFS_DEPRECATED_V2 but keep it for block devices"), so remove it. Signed-off-by: Gaosheng Cui Link: https://lore.kernel.org/r/20220909063337.1146151-1-cuigaosheng1@huawei.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/base.h | 2 -- 1 file changed, 2 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/base.h b/drivers/base/base.h index b3a43a164dcd..b902d1ecc247 100644 --- a/drivers/base/base.h +++ b/drivers/base/base.h @@ -154,8 +154,6 @@ extern void driver_remove_groups(struct device_driver *drv, const struct attribute_group **groups); void device_driver_detach(struct device *dev); -extern char *make_class_name(const char *name, struct kobject *kobj); - extern int devres_release_all(struct device *dev); extern void device_block_probing(void); extern void device_unblock_probing(void); -- cgit v1.2.3 From 01daccf748323dfc61112f474cf2ba81015446b0 Mon Sep 17 00:00:00 2001 From: Mukesh Ojha Date: Tue, 13 Sep 2022 18:20:24 +0530 Subject: devcoredump : Serialize devcd_del work In following scenario(diagram), when one thread X running dev_coredumpm() adds devcd device to the framework which sends uevent notification to userspace and another thread Y reads this uevent and call to devcd_data_write() which eventually try to delete the queued timer that is not initialized/queued yet. So, debug object reports some warning and in the meantime, timer is initialized and queued from X path. and from Y path, it gets reinitialized again and timer->entry.pprev=NULL and try_to_grab_pending() stucks. To fix this, introduce mutex and a boolean flag to serialize the behaviour. cpu0(X) cpu1(Y) dev_coredump() uevent sent to user space device_add() ======================> user space process Y reads the uevents writes to devcd fd which results into writes to devcd_data_write() mod_delayed_work() try_to_grab_pending() del_timer() debug_assert_init() INIT_DELAYED_WORK() schedule_delayed_work() debug_object_fixup() timer_fixup_assert_init() timer_setup() do_init_timer() /* Above call reinitializes the timer to timer->entry.pprev=NULL and this will be checked later in timer_pending() call. */ timer_pending() !hlist_unhashed_lockless(&timer->entry) !h->pprev /* del_timer() checks h->pprev and finds it to be NULL due to which try_to_grab_pending() stucks. */ Link: https://lore.kernel.org/lkml/2e1f81e2-428c-f11f-ce92-eb11048cb271@quicinc.com/ Signed-off-by: Mukesh Ojha Link: https://lore.kernel.org/r/1663073424-13663-1-git-send-email-quic_mojha@quicinc.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/devcoredump.c | 83 ++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 81 insertions(+), 2 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/devcoredump.c b/drivers/base/devcoredump.c index f4d794d6bb85..1c06781f7114 100644 --- a/drivers/base/devcoredump.c +++ b/drivers/base/devcoredump.c @@ -25,6 +25,47 @@ struct devcd_entry { struct device devcd_dev; void *data; size_t datalen; + /* + * Here, mutex is required to serialize the calls to del_wk work between + * user/kernel space which happens when devcd is added with device_add() + * and that sends uevent to user space. User space reads the uevents, + * and calls to devcd_data_write() which try to modify the work which is + * not even initialized/queued from devcoredump. + * + * + * + * cpu0(X) cpu1(Y) + * + * dev_coredump() uevent sent to user space + * device_add() ======================> user space process Y reads the + * uevents writes to devcd fd + * which results into writes to + * + * devcd_data_write() + * mod_delayed_work() + * try_to_grab_pending() + * del_timer() + * debug_assert_init() + * INIT_DELAYED_WORK() + * schedule_delayed_work() + * + * + * Also, mutex alone would not be enough to avoid scheduling of + * del_wk work after it get flush from a call to devcd_free() + * mentioned as below. + * + * disabled_store() + * devcd_free() + * mutex_lock() devcd_data_write() + * flush_delayed_work() + * mutex_unlock() + * mutex_lock() + * mod_delayed_work() + * mutex_unlock() + * So, delete_work flag is required. + */ + struct mutex mutex; + bool delete_work; struct module *owner; ssize_t (*read)(char *buffer, loff_t offset, size_t count, void *data, size_t datalen); @@ -84,7 +125,12 @@ static ssize_t devcd_data_write(struct file *filp, struct kobject *kobj, struct device *dev = kobj_to_dev(kobj); struct devcd_entry *devcd = dev_to_devcd(dev); - mod_delayed_work(system_wq, &devcd->del_wk, 0); + mutex_lock(&devcd->mutex); + if (!devcd->delete_work) { + devcd->delete_work = true; + mod_delayed_work(system_wq, &devcd->del_wk, 0); + } + mutex_unlock(&devcd->mutex); return count; } @@ -112,7 +158,12 @@ static int devcd_free(struct device *dev, void *data) { struct devcd_entry *devcd = dev_to_devcd(dev); + mutex_lock(&devcd->mutex); + if (!devcd->delete_work) + devcd->delete_work = true; + flush_delayed_work(&devcd->del_wk); + mutex_unlock(&devcd->mutex); return 0; } @@ -122,6 +173,30 @@ static ssize_t disabled_show(struct class *class, struct class_attribute *attr, return sysfs_emit(buf, "%d\n", devcd_disabled); } +/* + * + * disabled_store() worker() + * class_for_each_device(&devcd_class, + * NULL, NULL, devcd_free) + * ... + * ... + * while ((dev = class_dev_iter_next(&iter)) + * devcd_del() + * device_del() + * put_device() <- last reference + * error = fn(dev, data) devcd_dev_release() + * devcd_free(dev, data) kfree(devcd) + * mutex_lock(&devcd->mutex); + * + * + * In the above diagram, It looks like disabled_store() would be racing with parallely + * running devcd_del() and result in memory abort while acquiring devcd->mutex which + * is called after kfree of devcd memory after dropping its last reference with + * put_device(). However, this will not happens as fn(dev, data) runs + * with its own reference to device via klist_node so it is not its last reference. + * so, above situation would not occur. + */ + static ssize_t disabled_store(struct class *class, struct class_attribute *attr, const char *buf, size_t count) { @@ -278,13 +353,16 @@ void dev_coredumpm(struct device *dev, struct module *owner, devcd->read = read; devcd->free = free; devcd->failing_dev = get_device(dev); + devcd->delete_work = false; + mutex_init(&devcd->mutex); device_initialize(&devcd->devcd_dev); dev_set_name(&devcd->devcd_dev, "devcd%d", atomic_inc_return(&devcd_count)); devcd->devcd_dev.class = &devcd_class; + mutex_lock(&devcd->mutex); if (device_add(&devcd->devcd_dev)) goto put_device; @@ -301,10 +379,11 @@ void dev_coredumpm(struct device *dev, struct module *owner, INIT_DELAYED_WORK(&devcd->del_wk, devcd_del); schedule_delayed_work(&devcd->del_wk, DEVCD_TIMEOUT); - + mutex_unlock(&devcd->mutex); return; put_device: put_device(&devcd->devcd_dev); + mutex_unlock(&devcd->mutex); put_module: module_put(owner); free: -- cgit v1.2.3 From d70590d53a8996acf55426ba934d49eee9f091cd Mon Sep 17 00:00:00 2001 From: Yang Yingliang Date: Wed, 14 Sep 2022 22:07:53 +0800 Subject: driver core: use IS_ERR_OR_NULL() helper in device_create_groups_vargs() Use IS_ERR_OR_NULL() helper in device_create_groups_vargs() to simplify code and improve readiblity. No functional change. Signed-off-by: Yang Yingliang Link: https://lore.kernel.org/r/20220914140753.3799982-1-yangyingliang@huawei.com Signed-off-by: Greg Kroah-Hartman --- 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 e16840886f14..d02501933467 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -4170,7 +4170,7 @@ device_create_groups_vargs(struct class *class, struct device *parent, struct device *dev = NULL; int retval = -ENODEV; - if (class == NULL || IS_ERR(class)) + if (IS_ERR_OR_NULL(class)) goto error; dev = kzalloc(sizeof(*dev), GFP_KERNEL); -- cgit v1.2.3 From aade55c86033bee868a93e4bf3843c9c99e84526 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Thu, 22 Sep 2022 16:54:10 +0300 Subject: device property: Add const qualifier to device_get_match_data() parameter Add const qualifier to the device_get_match_data() parameter. Some of the future users may utilize this function without forcing the type. All the same, dev_fwnode() may be used with a const qualifier. Reported-by: kernel test robot Acked-by: Heikki Krogerus Signed-off-by: Andy Shevchenko Link: https://lore.kernel.org/r/20220922135410.49694-1-andriy.shevchenko@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/property.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/property.c b/drivers/base/property.c index ed6f449f8e5c..4d6278a84868 100644 --- a/drivers/base/property.c +++ b/drivers/base/property.c @@ -17,7 +17,7 @@ #include #include -struct fwnode_handle *dev_fwnode(struct device *dev) +struct fwnode_handle *dev_fwnode(const struct device *dev) { return IS_ENABLED(CONFIG_OF) && dev->of_node ? of_fwnode_handle(dev->of_node) : dev->fwnode; @@ -1200,7 +1200,7 @@ int fwnode_graph_parse_endpoint(const struct fwnode_handle *fwnode, } EXPORT_SYMBOL(fwnode_graph_parse_endpoint); -const void *device_get_match_data(struct device *dev) +const void *device_get_match_data(const struct device *dev) { return fwnode_call_ptr_op(dev_fwnode(dev), device_get_match_data, dev); } -- cgit v1.2.3