On Mon, 21 May 2018, Uwe Kleine-König wrote:
> > The race looks like that (thanks Jiri):
> >
> > 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* */
>
> [Context: For a patch to rtc-pcf2127.c Alexandre Belloni asked not to
> fail after rtc_device_register successfully finished and pointed to this
> reasoning as explaination.]
>
> If there is really such a race then (I hope) there is
> something in the cdev code that needs fixing. According to my
> understanding, when rtc_device_unregister returned, the cdev is gone and
> so chrdev_open is supposed to fail.
Oh wow, hello back to 4 years ago!
Looking at the current code, I don't think there is no such race any more,
as the last thing cmos_do_probe() -> __rtc_register_device() does that can
potentially fail is the chardev creation itself.
IOW if it exists, it's guaranteed to not go away on a probe error handling
path (and the rest is protected via ops_lock mutex).
--
Jiri Kosina
SUSE Labs
