From 25ece30561d247b2931b0d11d92e9c976a668771 Mon Sep 17 00:00:00 2001 From: Alexandre Belloni Date: Mon, 9 Nov 2020 17:34:05 +0100 Subject: rtc: nvmem: remove nvram ABI The nvram sysfs attributes have been deprecated at least since v4.13, more than 3 years ago and nobody ever complained about the deprecation warning. Remove the sysfs attributes now. [Bartosz: remove the declaration of rtc_nvmem_unregister()] Signed-off-by: Alexandre Belloni Signed-off-by: Bartosz Golaszewski Link: https://lore.kernel.org/r/20201109163409.24301-5-brgl@bgdev.pl --- drivers/rtc/class.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'drivers/rtc/class.c') diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c index 7c88d190c51f..a99b7d24b77c 100644 --- a/drivers/rtc/class.c +++ b/drivers/rtc/class.c @@ -339,8 +339,6 @@ static void devm_rtc_release_device(struct device *dev, void *res) { struct rtc_device *rtc = *(struct rtc_device **)res; - rtc_nvmem_unregister(rtc); - if (rtc->registered) rtc_device_unregister(rtc); else -- cgit From fdcfd854333be5b30377dc5daa9cd0fa1643a979 Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Mon, 9 Nov 2020 17:34:08 +0100 Subject: rtc: rework rtc_register_device() resource management rtc_register_device() is a managed interface but it doesn't use devres by itself - instead it marks an rtc_device as "registered" and the devres callback for devm_rtc_allocate_device() takes care of resource release. This doesn't correspond with the design behind devres where managed structures should not be aware of being managed. The correct solution here is to register a separate devres callback for unregistering the device. While at it: rename rtc_register_device() to devm_rtc_register_device() and add it to the list of managed interfaces in devres.rst. This way we can avoid any potential confusion of driver developers who may expect there to exist a corresponding unregister function. Signed-off-by: Bartosz Golaszewski Signed-off-by: Alexandre Belloni Link: https://lore.kernel.org/r/20201109163409.24301-8-brgl@bgdev.pl --- drivers/rtc/class.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) (limited to 'drivers/rtc/class.c') diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c index a99b7d24b77c..b8a34ee039ad 100644 --- a/drivers/rtc/class.c +++ b/drivers/rtc/class.c @@ -321,8 +321,10 @@ static void rtc_device_get_offset(struct rtc_device *rtc) * * @rtc: the RTC class device to destroy */ -static void rtc_device_unregister(struct rtc_device *rtc) +static void devm_rtc_unregister_device(void *data) { + struct rtc_device *rtc = data; + mutex_lock(&rtc->ops_lock); /* * Remove innards of this RTC, then disable it, before @@ -339,10 +341,7 @@ static void devm_rtc_release_device(struct device *dev, void *res) { struct rtc_device *rtc = *(struct rtc_device **)res; - if (rtc->registered) - rtc_device_unregister(rtc); - else - put_device(&rtc->dev); + put_device(&rtc->dev); } struct rtc_device *devm_rtc_allocate_device(struct device *dev) @@ -383,7 +382,7 @@ exit_ida: } EXPORT_SYMBOL_GPL(devm_rtc_allocate_device); -int __rtc_register_device(struct module *owner, struct rtc_device *rtc) +int __devm_rtc_register_device(struct module *owner, struct rtc_device *rtc) { struct rtc_wkalrm alrm; int err; @@ -413,7 +412,6 @@ int __rtc_register_device(struct module *owner, struct rtc_device *rtc) rtc_proc_add_device(rtc); - rtc->registered = true; dev_info(rtc->dev.parent, "registered as %s\n", dev_name(&rtc->dev)); @@ -422,9 +420,10 @@ int __rtc_register_device(struct module *owner, struct rtc_device *rtc) rtc_hctosys(rtc); #endif - return 0; + return devm_add_action_or_reset(rtc->dev.parent, + devm_rtc_unregister_device, rtc); } -EXPORT_SYMBOL_GPL(__rtc_register_device); +EXPORT_SYMBOL_GPL(__devm_rtc_register_device); /** * devm_rtc_device_register - resource managed rtc_device_register() @@ -454,7 +453,7 @@ struct rtc_device *devm_rtc_device_register(struct device *dev, rtc->ops = ops; - err = __rtc_register_device(owner, rtc); + err = __devm_rtc_register_device(owner, rtc); if (err) return ERR_PTR(err); -- cgit From 1bfc485b73579bff5326ac481fd9be7e24a5d5d1 Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Mon, 9 Nov 2020 17:34:09 +0100 Subject: rtc: shrink devm_rtc_allocate_device() We don't need to use devres_alloc() & devres_add() manually if all we want to manage is a single pointer. We can shrink the code by using devm_add_action_or_reset() instead. The number of allocations stays the same. Signed-off-by: Bartosz Golaszewski Signed-off-by: Alexandre Belloni Link: https://lore.kernel.org/r/20201109163409.24301-9-brgl@bgdev.pl --- drivers/rtc/class.c | 29 +++++++++-------------------- 1 file changed, 9 insertions(+), 20 deletions(-) (limited to 'drivers/rtc/class.c') diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c index b8a34ee039ad..a1b3711aaf01 100644 --- a/drivers/rtc/class.c +++ b/drivers/rtc/class.c @@ -337,48 +337,37 @@ static void devm_rtc_unregister_device(void *data) put_device(&rtc->dev); } -static void devm_rtc_release_device(struct device *dev, void *res) +static void devm_rtc_release_device(void *res) { - struct rtc_device *rtc = *(struct rtc_device **)res; + struct rtc_device *rtc = res; put_device(&rtc->dev); } struct rtc_device *devm_rtc_allocate_device(struct device *dev) { - struct rtc_device **ptr, *rtc; + struct rtc_device *rtc; int id, err; id = rtc_device_get_id(dev); if (id < 0) return ERR_PTR(id); - ptr = devres_alloc(devm_rtc_release_device, sizeof(*ptr), GFP_KERNEL); - if (!ptr) { - err = -ENOMEM; - goto exit_ida; - } - rtc = rtc_allocate_device(); if (!rtc) { - err = -ENOMEM; - goto exit_devres; + ida_simple_remove(&rtc_ida, id); + return ERR_PTR(-ENOMEM); } - *ptr = rtc; - devres_add(dev, ptr); - rtc->id = id; rtc->dev.parent = dev; dev_set_name(&rtc->dev, "rtc%d", id); - return rtc; + err = devm_add_action_or_reset(dev, devm_rtc_release_device, rtc); + if (err) + return ERR_PTR(err); -exit_devres: - devres_free(ptr); -exit_ida: - ida_simple_remove(&rtc_ida, id); - return ERR_PTR(err); + return rtc; } EXPORT_SYMBOL_GPL(devm_rtc_allocate_device); -- cgit From 0d6d7a390b32ef23d957960d3bb8586a49d6af7c Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Tue, 10 Nov 2020 10:42:05 +0100 Subject: rtc: destroy mutex when releasing the device Not destroying mutexes doesn't lead to resource leak but it's the correct thing to do for mutex debugging accounting. Signed-off-by: Bartosz Golaszewski Signed-off-by: Alexandre Belloni Link: https://lore.kernel.org/r/20201110094205.8972-1-brgl@bgdev.pl --- drivers/rtc/class.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/rtc/class.c') diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c index a1b3711aaf01..e6b44b7c4ad3 100644 --- a/drivers/rtc/class.c +++ b/drivers/rtc/class.c @@ -28,6 +28,7 @@ static void rtc_device_release(struct device *dev) struct rtc_device *rtc = to_rtc_device(dev); ida_simple_remove(&rtc_ida, rtc->id); + mutex_destroy(&rtc->ops_lock); kfree(rtc); } -- cgit From f70cc33029fca3cf62bffb15102ea42eb4d097ac Mon Sep 17 00:00:00 2001 From: Alexandre Belloni Date: Sun, 6 Dec 2020 00:14:48 +0100 Subject: rtc: fix RTC removal Since the rtc_register_device, removing an RTC device will end with a refcount_t: underflow; use-after-free warning since put_device is called twice in the device tear down path. Fixes: fdcfd854333b ("rtc: rework rtc_register_device() resource management") Signed-off-by: Alexandre Belloni Reviewed-by: Bartosz Golaszewski Link: https://lore.kernel.org/r/20201205231449.610980-1-alexandre.belloni@bootlin.com --- drivers/rtc/class.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers/rtc/class.c') diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c index e6b44b7c4ad3..5c6748dfa55d 100644 --- a/drivers/rtc/class.c +++ b/drivers/rtc/class.c @@ -335,7 +335,6 @@ static void devm_rtc_unregister_device(void *data) cdev_device_del(&rtc->char_dev, &rtc->dev); rtc->ops = NULL; mutex_unlock(&rtc->ops_lock); - put_device(&rtc->dev); } static void devm_rtc_release_device(void *res) -- cgit