Hi Damien,
On 11/25/2015 09:09 AM, Damien Riegel wrote:
On Tue, Nov 24, 2015 at 06:20:11PM -0800, Guenter Roeck wrote:
On 11/24/2015 03:45 PM, Damien Riegel wrote:
device_create is called after watchdog_dev_register, so it makes more
sense to call the cleanup functions in reverse order, ie. device_destroy
before watchdog_dev_unregister.
Signed-off-by: Damien Riegel <damien.riegel@xxxxxxxxxxxxxxxxxxxx>
Reviewed-by: Guenter Roeck <linux@xxxxxxxxxxxx>
On second thought, I am wondering if the proper fix would not be to call
device_create before watchdog_dev_register. Consider the following
scenario:
watchdog_register_device
__watchdog_register_device
watchdog_dev_register returns successfully, char dev is live
device_create fails, setting wdd->dev to an ERR_PTR
...
meanwhile, a user opens the watchdog, hence ops->start is called.
If ops->start uses wdd->dev (to print a debug message for
instance), it will dereference an invalid pointer.
Admittedly, it should be quite rare, but there is still a chance for a
race condition here.
Only we should not have race conditions, and this might actually happen
if user space listens for a udev event on the character device, and device
creation is delayed for some reasons.
I think you are right, that is a problem. Back to the drawing board.
Ok, next question: Does it hurt to call device_create() first ?
That creates the sysfs entries for the driver.
If that doesn't work either, the only other idea I have would be to reject
an attempt to open the character device with -EAGAIN or similar if the
device node is not yet (or not anymore) available. Or maybe that would
be the correct approach anyway ? Or can we use some lock to synchronize
the two operations ?
Thanks,
Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
