From 53d29e0a3afe0567db9e4360e8523b092eb2d4e4 Mon Sep 17 00:00:00 2001 From: Alexandre Belloni Date: Mon, 12 Feb 2018 23:47:45 +0100 Subject: rtc: cmos: fix possible race condition The probe function is not allowed to fail after registering the RTC because the following may happen: CPU0: CPU1: sys_load_module() do_init_module() do_one_initcall() cmos_do_probe() rtc_device_register() __register_chrdev() cdev->owner = struct module* open("/dev/rtc0") rtc_device_unregister() module_put() free_module() module_free(mod->module_core) /* struct module *module is now freed */ chrdev_open() spin_lock(cdev_lock) cdev_get() try_module_get() module_is_live() /* dereferences already freed struct module* */ Switch to devm_rtc_allocate_device/rtc_register_device to register the rtc as late as possible. Signed-off-by: Alexandre Belloni --- drivers/rtc/rtc-cmos.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'drivers/rtc/rtc-cmos.c') diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c index 9dca53df3584..e6393e784d0c 100644 --- a/drivers/rtc/rtc-cmos.c +++ b/drivers/rtc/rtc-cmos.c @@ -751,8 +751,7 @@ cmos_do_probe(struct device *dev, struct resource *ports, int rtc_irq) cmos_rtc.dev = dev; dev_set_drvdata(dev, &cmos_rtc); - cmos_rtc.rtc = rtc_device_register(driver_name, dev, - &cmos_rtc_ops, THIS_MODULE); + cmos_rtc.rtc = devm_rtc_allocate_device(dev); if (IS_ERR(cmos_rtc.rtc)) { retval = PTR_ERR(cmos_rtc.rtc); goto cleanup0; @@ -822,6 +821,11 @@ cmos_do_probe(struct device *dev, struct resource *ports, int rtc_irq) goto cleanup2; } + cmos_rtc.rtc->ops = &cmos_rtc_ops; + retval = rtc_register_device(cmos_rtc.rtc); + if (retval) + goto cleanup3; + dev_info(dev, "%s%s, %zd bytes nvram%s\n", !is_valid_irq(rtc_irq) ? "no alarms" : cmos_rtc.mon_alrm ? "alarms up to one year" : @@ -833,12 +837,13 @@ cmos_do_probe(struct device *dev, struct resource *ports, int rtc_irq) return 0; +cleanup3: + sysfs_remove_bin_file(&dev->kobj, &nvram); cleanup2: if (is_valid_irq(rtc_irq)) free_irq(rtc_irq, cmos_rtc.rtc); cleanup1: cmos_rtc.dev = NULL; - rtc_device_unregister(cmos_rtc.rtc); cleanup0: if (RTC_IOMAPPED) release_region(ports->start, resource_size(ports)); @@ -869,7 +874,6 @@ static void cmos_do_remove(struct device *dev) hpet_unregister_irq_handler(cmos_interrupt); } - rtc_device_unregister(cmos->rtc); cmos->rtc = NULL; ports = cmos->iomem; -- cgit v1.2.3 From 8b5b7958fd1cac54bdca62ec5552c6be0b38def4 Mon Sep 17 00:00:00 2001 From: Alexandre Belloni Date: Mon, 12 Feb 2018 23:47:46 +0100 Subject: rtc: cmos: use generic nvmem Instead of adding a binary sysfs attribute from the driver, use the core to register an nvmem device. This allows to use the in-kernel interface to access the nvram. Signed-off-by: Alexandre Belloni --- drivers/rtc/rtc-cmos.c | 73 +++++++++++++++++++++----------------------------- 1 file changed, 30 insertions(+), 43 deletions(-) (limited to 'drivers/rtc/rtc-cmos.c') diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c index e6393e784d0c..b8ec6009171a 100644 --- a/drivers/rtc/rtc-cmos.c +++ b/drivers/rtc/rtc-cmos.c @@ -541,11 +541,10 @@ static const struct rtc_class_ops cmos_rtc_ops = { #define NVRAM_OFFSET (RTC_REG_D + 1) -static ssize_t -cmos_nvram_read(struct file *filp, struct kobject *kobj, - struct bin_attribute *attr, - char *buf, loff_t off, size_t count) +static int cmos_nvram_read(void *priv, unsigned int off, void *val, + size_t count) { + unsigned char *buf = val; int retval; off += NVRAM_OFFSET; @@ -563,16 +562,13 @@ cmos_nvram_read(struct file *filp, struct kobject *kobj, return retval; } -static ssize_t -cmos_nvram_write(struct file *filp, struct kobject *kobj, - struct bin_attribute *attr, - char *buf, loff_t off, size_t count) +static int cmos_nvram_write(void *priv, unsigned int off, void *val, + size_t count) { - struct cmos_rtc *cmos; + struct cmos_rtc *cmos = priv; + unsigned char *buf = val; int retval; - cmos = dev_get_drvdata(container_of(kobj, struct device, kobj)); - /* NOTE: on at least PCs and Ataris, the boot firmware uses a * checksum on part of the NVRAM data. That's currently ignored * here. If userspace is smart enough to know what fields of @@ -598,17 +594,6 @@ cmos_nvram_write(struct file *filp, struct kobject *kobj, return retval; } -static struct bin_attribute nvram = { - .attr = { - .name = "nvram", - .mode = S_IRUGO | S_IWUSR, - }, - - .read = cmos_nvram_read, - .write = cmos_nvram_write, - /* size gets set up later */ -}; - /*----------------------------------------------------------------*/ static struct cmos_rtc cmos_rtc; @@ -675,6 +660,14 @@ cmos_do_probe(struct device *dev, struct resource *ports, int rtc_irq) unsigned char rtc_control; unsigned address_space; u32 flags = 0; + struct nvmem_config nvmem_cfg = { + .name = "cmos_nvram", + .word_size = 1, + .stride = 1, + .reg_read = cmos_nvram_read, + .reg_write = cmos_nvram_write, + .priv = &cmos_rtc, + }; /* there can be only one ... */ if (cmos_rtc.dev) @@ -813,32 +806,28 @@ cmos_do_probe(struct device *dev, struct resource *ports, int rtc_irq) } } - /* export at least the first block of NVRAM */ - nvram.size = address_space - NVRAM_OFFSET; - retval = sysfs_create_bin_file(&dev->kobj, &nvram); - if (retval < 0) { - dev_dbg(dev, "can't create nvram file? %d\n", retval); - goto cleanup2; - } - cmos_rtc.rtc->ops = &cmos_rtc_ops; + cmos_rtc.rtc->nvram_old_abi = true; retval = rtc_register_device(cmos_rtc.rtc); if (retval) - goto cleanup3; + goto cleanup2; - dev_info(dev, "%s%s, %zd bytes nvram%s\n", - !is_valid_irq(rtc_irq) ? "no alarms" : - cmos_rtc.mon_alrm ? "alarms up to one year" : - cmos_rtc.day_alrm ? "alarms up to one month" : - "alarms up to one day", - cmos_rtc.century ? ", y3k" : "", - nvram.size, - is_hpet_enabled() ? ", hpet irqs" : ""); + /* export at least the first block of NVRAM */ + nvmem_cfg.size = address_space - NVRAM_OFFSET; + if (rtc_nvmem_register(cmos_rtc.rtc, &nvmem_cfg)) + dev_err(dev, "nvmem registration failed\n"); + + dev_info(dev, "%s%s, %d bytes nvram%s\n", + !is_valid_irq(rtc_irq) ? "no alarms" : + cmos_rtc.mon_alrm ? "alarms up to one year" : + cmos_rtc.day_alrm ? "alarms up to one month" : + "alarms up to one day", + cmos_rtc.century ? ", y3k" : "", + nvmem_cfg.size, + is_hpet_enabled() ? ", hpet irqs" : ""); return 0; -cleanup3: - sysfs_remove_bin_file(&dev->kobj, &nvram); cleanup2: if (is_valid_irq(rtc_irq)) free_irq(rtc_irq, cmos_rtc.rtc); @@ -867,8 +856,6 @@ static void cmos_do_remove(struct device *dev) cmos_do_shutdown(cmos->irq); - sysfs_remove_bin_file(&dev->kobj, &nvram); - if (is_valid_irq(cmos->irq)) { free_irq(cmos->irq, cmos->rtc); hpet_unregister_irq_handler(cmos_interrupt); -- cgit v1.2.3 From 812318a094d0715194d9f686b22ee67e7dc59d93 Mon Sep 17 00:00:00 2001 From: Alexandre Belloni Date: Wed, 21 Feb 2018 11:44:26 +0100 Subject: rtc: cmos: let the core handle invalid time Setting the rtc to a valid time when the time is invalid is a bad practice, because then userspace doesn't know it shouldn't trust the RTC. Signed-off-by: Alexandre Belloni --- drivers/rtc/rtc-cmos.c | 12 ------------ 1 file changed, 12 deletions(-) (limited to 'drivers/rtc/rtc-cmos.c') diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c index b8ec6009171a..d98ad4874d8b 100644 --- a/drivers/rtc/rtc-cmos.c +++ b/drivers/rtc/rtc-cmos.c @@ -1262,8 +1262,6 @@ MODULE_DEVICE_TABLE(of, of_cmos_match); static __init void cmos_of_init(struct platform_device *pdev) { struct device_node *node = pdev->dev.of_node; - struct rtc_time time; - int ret; const __be32 *val; if (!node) @@ -1276,16 +1274,6 @@ static __init void cmos_of_init(struct platform_device *pdev) val = of_get_property(node, "freq-reg", NULL); if (val) CMOS_WRITE(be32_to_cpup(val), RTC_FREQ_SELECT); - - cmos_read_time(&pdev->dev, &time); - ret = rtc_valid_tm(&time); - if (ret) { - struct rtc_time def_time = { - .tm_year = 1, - .tm_mday = 1, - }; - cmos_set_time(&pdev->dev, &def_time); - } } #else static inline void cmos_of_init(struct platform_device *pdev) {} -- cgit v1.2.3