[adding linux-rtc ML and Alexandre to Cc:]
Hello,
On Tue, May 22, 2018 at 02:09:36PM +0200, Jiri Kosina wrote:
> 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.
OK, so you agree that it's also save to do something in a driver's probe
after rtc_device_register() and call rtc_device_unregister() in the error
path, right? (That's the motivation for me to discuss this old topic.)
> 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).
Putting it in other words (assuming I got you right):
If there is a race, it needs fixing in the cdev code. The rtc driver can
stay as is.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
