* [4.14-stable] watchdog: Fix the race between the release of watchdog_core_data and cdev
@ 2020-05-20 20:10 Ben Hutchings
2020-05-20 20:15 ` [4.9-stable] " Ben Hutchings
0 siblings, 1 reply; 4+ messages in thread
From: Ben Hutchings @ 2020-05-20 20:10 UTC (permalink / raw)
To: Greg Kroah-Hartman, Sasha Levin; +Cc: stable
[-- Attachment #1: Type: text/plain, Size: 374 bytes --]
Please queue up the attached backport of commit 2351c88f8296 "watchdog:
Fix the race between the release of watchdog_core_data and cdev" for
4.14.
Ben.
--
Ben Hutchings, Software Developer Codethink Ltd
https://www.codethink.co.uk/ Dale House, 35 Dale Street
Manchester, M1 2HF, United Kingdom
[-- Attachment #2: 0001-watchdog-Fix-the-race-between-the-release-of-watchdo.patch --]
[-- Type: text/x-patch, Size: 9768 bytes --]
From 2351c88f8296ef95e1ceda815c229e57ac33ea64 Mon Sep 17 00:00:00 2001
From: Kevin Hao <haokexin@gmail.com>
Date: Tue, 8 Oct 2019 19:29:34 +0800
Subject: watchdog: Fix the race between the release of watchdog_core_data and
cdev
commit 72139dfa2464e43957d330266994740bb7be2535 upstream.
The struct cdev is embedded in the struct watchdog_core_data. In the
current code, we manage the watchdog_core_data with a kref, but the
cdev is manged by a kobject. There is no any relationship between
this kref and kobject. So it is possible that the watchdog_core_data is
freed before the cdev is entirely released. We can easily get the
following call trace with CONFIG_DEBUG_KOBJECT_RELEASE and
CONFIG_DEBUG_OBJECTS_TIMERS enabled.
ODEBUG: free active (active state 0) object type: timer_list hint: delayed_work_timer_fn+0x0/0x38
WARNING: CPU: 23 PID: 1028 at lib/debugobjects.c:481 debug_print_object+0xb0/0xf0
Modules linked in: softdog(-) deflate ctr twofish_generic twofish_common camellia_generic serpent_generic blowfish_generic blowfish_common cast5_generic cast_common cmac xcbc af_key sch_fq_codel openvswitch nsh nf_conncount nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4
CPU: 23 PID: 1028 Comm: modprobe Not tainted 5.3.0-next-20190924-yoctodev-standard+ #180
Hardware name: Marvell OcteonTX CN96XX board (DT)
pstate: 00400009 (nzcv daif +PAN -UAO)
pc : debug_print_object+0xb0/0xf0
lr : debug_print_object+0xb0/0xf0
sp : ffff80001cbcfc70
x29: ffff80001cbcfc70 x28: ffff800010ea2128
x27: ffff800010bad000 x26: 0000000000000000
x25: ffff80001103c640 x24: ffff80001107b268
x23: ffff800010bad9e8 x22: ffff800010ea2128
x21: ffff000bc2c62af8 x20: ffff80001103c600
x19: ffff800010e867d8 x18: 0000000000000060
x17: 0000000000000000 x16: 0000000000000000
x15: ffff000bd7240470 x14: 6e6968207473696c
x13: 5f72656d6974203a x12: 6570797420746365
x11: 6a626f2029302065 x10: 7461747320657669
x9 : 7463612820657669 x8 : 3378302f3078302b
x7 : 0000000000001d7a x6 : ffff800010fd5889
x5 : 0000000000000000 x4 : 0000000000000000
x3 : 0000000000000000 x2 : ffff000bff948548
x1 : 276a1c9e1edc2300 x0 : 0000000000000000
Call trace:
debug_print_object+0xb0/0xf0
debug_check_no_obj_freed+0x1e8/0x210
kfree+0x1b8/0x368
watchdog_cdev_unregister+0x88/0xc8
watchdog_dev_unregister+0x38/0x48
watchdog_unregister_device+0xa8/0x100
softdog_exit+0x18/0xfec4 [softdog]
__arm64_sys_delete_module+0x174/0x200
el0_svc_handler+0xd0/0x1c8
el0_svc+0x8/0xc
This is a common issue when using cdev embedded in a struct.
Fortunately, we already have a mechanism to solve this kind of issue.
Please see commit 233ed09d7fda ("chardev: add helper function to
register char devs with a struct device") for more detail.
In this patch, we choose to embed the struct device into the
watchdog_core_data, and use the API provided by the commit 233ed09d7fda
to make sure that the release of watchdog_core_data and cdev are
in sequence.
Signed-off-by: Kevin Hao <haokexin@gmail.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Link: https://lore.kernel.org/r/20191008112934.29669-1-haokexin@gmail.com
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Wim Van Sebroeck <wim@linux-watchdog.org>
[bwh: Backported to 4.14:
- There's no reboot notifier here
- Adjust context]
Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
---
drivers/watchdog/watchdog_dev.c | 67 +++++++++++++++------------------
1 file changed, 30 insertions(+), 37 deletions(-)
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 52e03f1c76e3..21c3ffdc8a09 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -38,7 +38,6 @@
#include <linux/init.h> /* For __init/__exit/... */
#include <linux/jiffies.h> /* For timeout functions */
#include <linux/kernel.h> /* For printk/panic/... */
-#include <linux/kref.h> /* For data references */
#include <linux/miscdevice.h> /* For handling misc devices */
#include <linux/module.h> /* For module stuff/... */
#include <linux/mutex.h> /* For mutexes */
@@ -53,14 +52,14 @@
/*
* struct watchdog_core_data - watchdog core internal data
- * @kref: Reference count.
+ * @dev: The watchdog's internal device
* @cdev: The watchdog's Character device.
* @wdd: Pointer to watchdog device.
* @lock: Lock for watchdog core.
* @status: Watchdog core internal status bits.
*/
struct watchdog_core_data {
- struct kref kref;
+ struct device dev;
struct cdev cdev;
struct watchdog_device *wdd;
struct mutex lock;
@@ -802,7 +801,7 @@ static int watchdog_open(struct inode *inode, struct file *file)
file->private_data = wd_data;
if (!hw_running)
- kref_get(&wd_data->kref);
+ get_device(&wd_data->dev);
/* dev/watchdog is a virtual (and thus non-seekable) filesystem */
return nonseekable_open(inode, file);
@@ -814,11 +813,11 @@ static int watchdog_open(struct inode *inode, struct file *file)
return err;
}
-static void watchdog_core_data_release(struct kref *kref)
+static void watchdog_core_data_release(struct device *dev)
{
struct watchdog_core_data *wd_data;
- wd_data = container_of(kref, struct watchdog_core_data, kref);
+ wd_data = container_of(dev, struct watchdog_core_data, dev);
kfree(wd_data);
}
@@ -878,7 +877,7 @@ static int watchdog_release(struct inode *inode, struct file *file)
*/
if (!running) {
module_put(wd_data->cdev.owner);
- kref_put(&wd_data->kref, watchdog_core_data_release);
+ put_device(&wd_data->dev);
}
return 0;
}
@@ -897,17 +896,22 @@ static struct miscdevice watchdog_miscdev = {
.fops = &watchdog_fops,
};
+static struct class watchdog_class = {
+ .name = "watchdog",
+ .owner = THIS_MODULE,
+ .dev_groups = wdt_groups,
+};
+
/*
* watchdog_cdev_register: register watchdog character device
* @wdd: watchdog device
- * @devno: character device number
*
* Register a watchdog character device including handling the legacy
* /dev/watchdog node. /dev/watchdog is actually a miscdevice and
* thus we set it up like that.
*/
-static int watchdog_cdev_register(struct watchdog_device *wdd, dev_t devno)
+static int watchdog_cdev_register(struct watchdog_device *wdd)
{
struct watchdog_core_data *wd_data;
int err;
@@ -915,7 +919,6 @@ static int watchdog_cdev_register(struct watchdog_device *wdd, dev_t devno)
wd_data = kzalloc(sizeof(struct watchdog_core_data), GFP_KERNEL);
if (!wd_data)
return -ENOMEM;
- kref_init(&wd_data->kref);
mutex_init(&wd_data->lock);
wd_data->wdd = wdd;
@@ -942,23 +945,33 @@ static int watchdog_cdev_register(struct watchdog_device *wdd, dev_t devno)
}
}
+ device_initialize(&wd_data->dev);
+ wd_data->dev.devt = MKDEV(MAJOR(watchdog_devt), wdd->id);
+ wd_data->dev.class = &watchdog_class;
+ wd_data->dev.parent = wdd->parent;
+ wd_data->dev.groups = wdd->groups;
+ wd_data->dev.release = watchdog_core_data_release;
+ dev_set_drvdata(&wd_data->dev, wdd);
+ dev_set_name(&wd_data->dev, "watchdog%d", wdd->id);
+
/* Fill in the data structures */
cdev_init(&wd_data->cdev, &watchdog_fops);
- wd_data->cdev.owner = wdd->ops->owner;
/* Add the device */
- err = cdev_add(&wd_data->cdev, devno, 1);
+ err = cdev_device_add(&wd_data->cdev, &wd_data->dev);
if (err) {
pr_err("watchdog%d unable to add device %d:%d\n",
wdd->id, MAJOR(watchdog_devt), wdd->id);
if (wdd->id == 0) {
misc_deregister(&watchdog_miscdev);
old_wd_data = NULL;
- kref_put(&wd_data->kref, watchdog_core_data_release);
+ put_device(&wd_data->dev);
}
return err;
}
+ wd_data->cdev.owner = wdd->ops->owner;
+
/* Record time of most recent heartbeat as 'just before now'. */
wd_data->last_hw_keepalive = jiffies - 1;
@@ -968,7 +981,7 @@ static int watchdog_cdev_register(struct watchdog_device *wdd, dev_t devno)
*/
if (watchdog_hw_running(wdd)) {
__module_get(wdd->ops->owner);
- kref_get(&wd_data->kref);
+ get_device(&wd_data->dev);
if (handle_boot_enabled)
queue_delayed_work(watchdog_wq, &wd_data->work, 0);
else
@@ -991,7 +1004,7 @@ static void watchdog_cdev_unregister(struct watchdog_device *wdd)
{
struct watchdog_core_data *wd_data = wdd->wd_data;
- cdev_del(&wd_data->cdev);
+ cdev_device_del(&wd_data->cdev, &wd_data->dev);
if (wdd->id == 0) {
misc_deregister(&watchdog_miscdev);
old_wd_data = NULL;
@@ -1009,15 +1022,9 @@ static void watchdog_cdev_unregister(struct watchdog_device *wdd)
cancel_delayed_work_sync(&wd_data->work);
- kref_put(&wd_data->kref, watchdog_core_data_release);
+ put_device(&wd_data->dev);
}
-static struct class watchdog_class = {
- .name = "watchdog",
- .owner = THIS_MODULE,
- .dev_groups = wdt_groups,
-};
-
/*
* watchdog_dev_register: register a watchdog device
* @wdd: watchdog device
@@ -1029,27 +1036,14 @@ static struct class watchdog_class = {
int watchdog_dev_register(struct watchdog_device *wdd)
{
- struct device *dev;
- dev_t devno;
int ret;
- devno = MKDEV(MAJOR(watchdog_devt), wdd->id);
-
- ret = watchdog_cdev_register(wdd, devno);
+ ret = watchdog_cdev_register(wdd);
if (ret)
return ret;
- dev = device_create_with_groups(&watchdog_class, wdd->parent,
- devno, wdd, wdd->groups,
- "watchdog%d", wdd->id);
- if (IS_ERR(dev)) {
- watchdog_cdev_unregister(wdd);
- return PTR_ERR(dev);
- }
-
ret = watchdog_register_pretimeout(wdd);
if (ret) {
- device_destroy(&watchdog_class, devno);
watchdog_cdev_unregister(wdd);
}
@@ -1067,7 +1061,6 @@ int watchdog_dev_register(struct watchdog_device *wdd)
void watchdog_dev_unregister(struct watchdog_device *wdd)
{
watchdog_unregister_pretimeout(wdd);
- device_destroy(&watchdog_class, wdd->wd_data->cdev.dev);
watchdog_cdev_unregister(wdd);
}
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [4.9-stable] watchdog: Fix the race between the release of watchdog_core_data and cdev
2020-05-20 20:10 [4.14-stable] watchdog: Fix the race between the release of watchdog_core_data and cdev Ben Hutchings
@ 2020-05-20 20:15 ` Ben Hutchings
2020-05-21 5:45 ` Greg Kroah-Hartman
2020-05-21 5:49 ` Greg Kroah-Hartman
0 siblings, 2 replies; 4+ messages in thread
From: Ben Hutchings @ 2020-05-20 20:15 UTC (permalink / raw)
To: Greg Kroah-Hartman, Sasha Levin; +Cc: stable
[-- Attachment #1: Type: text/plain, Size: 483 bytes --]
On Wed, 2020-05-20 at 21:10 +0100, Ben Hutchings wrote:
> Please queue up the attached backport of commit 2351c88f8296 "watchdog:
> Fix the race between the release of watchdog_core_data and cdev" for
> 4.14.
And here's the corresponding version for 4.9.
Ben.
--
Ben Hutchings, Software Developer Codethink Ltd
https://www.codethink.co.uk/ Dale House, 35 Dale Street
Manchester, M1 2HF, United Kingdom
[-- Attachment #2: 0001-watchdog-Fix-the-race-between-the-release-of-watchdo.patch --]
[-- Type: text/x-patch, Size: 9734 bytes --]
From 1cf1b24c844a037da38e6096a865bcab75aa05eb Mon Sep 17 00:00:00 2001
From: Kevin Hao <haokexin@gmail.com>
Date: Tue, 8 Oct 2019 19:29:34 +0800
Subject: watchdog: Fix the race between the release of watchdog_core_data and
cdev
commit 72139dfa2464e43957d330266994740bb7be2535 upstream.
The struct cdev is embedded in the struct watchdog_core_data. In the
current code, we manage the watchdog_core_data with a kref, but the
cdev is manged by a kobject. There is no any relationship between
this kref and kobject. So it is possible that the watchdog_core_data is
freed before the cdev is entirely released. We can easily get the
following call trace with CONFIG_DEBUG_KOBJECT_RELEASE and
CONFIG_DEBUG_OBJECTS_TIMERS enabled.
ODEBUG: free active (active state 0) object type: timer_list hint: delayed_work_timer_fn+0x0/0x38
WARNING: CPU: 23 PID: 1028 at lib/debugobjects.c:481 debug_print_object+0xb0/0xf0
Modules linked in: softdog(-) deflate ctr twofish_generic twofish_common camellia_generic serpent_generic blowfish_generic blowfish_common cast5_generic cast_common cmac xcbc af_key sch_fq_codel openvswitch nsh nf_conncount nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4
CPU: 23 PID: 1028 Comm: modprobe Not tainted 5.3.0-next-20190924-yoctodev-standard+ #180
Hardware name: Marvell OcteonTX CN96XX board (DT)
pstate: 00400009 (nzcv daif +PAN -UAO)
pc : debug_print_object+0xb0/0xf0
lr : debug_print_object+0xb0/0xf0
sp : ffff80001cbcfc70
x29: ffff80001cbcfc70 x28: ffff800010ea2128
x27: ffff800010bad000 x26: 0000000000000000
x25: ffff80001103c640 x24: ffff80001107b268
x23: ffff800010bad9e8 x22: ffff800010ea2128
x21: ffff000bc2c62af8 x20: ffff80001103c600
x19: ffff800010e867d8 x18: 0000000000000060
x17: 0000000000000000 x16: 0000000000000000
x15: ffff000bd7240470 x14: 6e6968207473696c
x13: 5f72656d6974203a x12: 6570797420746365
x11: 6a626f2029302065 x10: 7461747320657669
x9 : 7463612820657669 x8 : 3378302f3078302b
x7 : 0000000000001d7a x6 : ffff800010fd5889
x5 : 0000000000000000 x4 : 0000000000000000
x3 : 0000000000000000 x2 : ffff000bff948548
x1 : 276a1c9e1edc2300 x0 : 0000000000000000
Call trace:
debug_print_object+0xb0/0xf0
debug_check_no_obj_freed+0x1e8/0x210
kfree+0x1b8/0x368
watchdog_cdev_unregister+0x88/0xc8
watchdog_dev_unregister+0x38/0x48
watchdog_unregister_device+0xa8/0x100
softdog_exit+0x18/0xfec4 [softdog]
__arm64_sys_delete_module+0x174/0x200
el0_svc_handler+0xd0/0x1c8
el0_svc+0x8/0xc
This is a common issue when using cdev embedded in a struct.
Fortunately, we already have a mechanism to solve this kind of issue.
Please see commit 233ed09d7fda ("chardev: add helper function to
register char devs with a struct device") for more detail.
In this patch, we choose to embed the struct device into the
watchdog_core_data, and use the API provided by the commit 233ed09d7fda
to make sure that the release of watchdog_core_data and cdev are
in sequence.
Signed-off-by: Kevin Hao <haokexin@gmail.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Link: https://lore.kernel.org/r/20191008112934.29669-1-haokexin@gmail.com
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Wim Van Sebroeck <wim@linux-watchdog.org>
[bwh: Backported to 4.9:
- There's no reboot notifier here
- Adjust context]
Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
---
drivers/watchdog/watchdog_dev.c | 67 +++++++++++++++------------------
1 file changed, 30 insertions(+), 37 deletions(-)
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 9e17d933ea94..3167f276c4c2 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -38,7 +38,6 @@
#include <linux/init.h> /* For __init/__exit/... */
#include <linux/jiffies.h> /* For timeout functions */
#include <linux/kernel.h> /* For printk/panic/... */
-#include <linux/kref.h> /* For data references */
#include <linux/miscdevice.h> /* For handling misc devices */
#include <linux/module.h> /* For module stuff/... */
#include <linux/mutex.h> /* For mutexes */
@@ -53,14 +52,14 @@
/*
* struct watchdog_core_data - watchdog core internal data
- * @kref: Reference count.
+ * @dev: The watchdog's internal device
* @cdev: The watchdog's Character device.
* @wdd: Pointer to watchdog device.
* @lock: Lock for watchdog core.
* @status: Watchdog core internal status bits.
*/
struct watchdog_core_data {
- struct kref kref;
+ struct device dev;
struct cdev cdev;
struct watchdog_device *wdd;
struct mutex lock;
@@ -794,7 +793,7 @@ static int watchdog_open(struct inode *inode, struct file *file)
file->private_data = wd_data;
if (!hw_running)
- kref_get(&wd_data->kref);
+ get_device(&wd_data->dev);
/* dev/watchdog is a virtual (and thus non-seekable) filesystem */
return nonseekable_open(inode, file);
@@ -806,11 +805,11 @@ static int watchdog_open(struct inode *inode, struct file *file)
return err;
}
-static void watchdog_core_data_release(struct kref *kref)
+static void watchdog_core_data_release(struct device *dev)
{
struct watchdog_core_data *wd_data;
- wd_data = container_of(kref, struct watchdog_core_data, kref);
+ wd_data = container_of(dev, struct watchdog_core_data, dev);
kfree(wd_data);
}
@@ -870,7 +869,7 @@ static int watchdog_release(struct inode *inode, struct file *file)
*/
if (!running) {
module_put(wd_data->cdev.owner);
- kref_put(&wd_data->kref, watchdog_core_data_release);
+ put_device(&wd_data->dev);
}
return 0;
}
@@ -889,17 +888,22 @@ static struct miscdevice watchdog_miscdev = {
.fops = &watchdog_fops,
};
+static struct class watchdog_class = {
+ .name = "watchdog",
+ .owner = THIS_MODULE,
+ .dev_groups = wdt_groups,
+};
+
/*
* watchdog_cdev_register: register watchdog character device
* @wdd: watchdog device
- * @devno: character device number
*
* Register a watchdog character device including handling the legacy
* /dev/watchdog node. /dev/watchdog is actually a miscdevice and
* thus we set it up like that.
*/
-static int watchdog_cdev_register(struct watchdog_device *wdd, dev_t devno)
+static int watchdog_cdev_register(struct watchdog_device *wdd)
{
struct watchdog_core_data *wd_data;
int err;
@@ -907,7 +911,6 @@ static int watchdog_cdev_register(struct watchdog_device *wdd, dev_t devno)
wd_data = kzalloc(sizeof(struct watchdog_core_data), GFP_KERNEL);
if (!wd_data)
return -ENOMEM;
- kref_init(&wd_data->kref);
mutex_init(&wd_data->lock);
wd_data->wdd = wdd;
@@ -934,23 +937,33 @@ static int watchdog_cdev_register(struct watchdog_device *wdd, dev_t devno)
}
}
+ device_initialize(&wd_data->dev);
+ wd_data->dev.devt = MKDEV(MAJOR(watchdog_devt), wdd->id);
+ wd_data->dev.class = &watchdog_class;
+ wd_data->dev.parent = wdd->parent;
+ wd_data->dev.groups = wdd->groups;
+ wd_data->dev.release = watchdog_core_data_release;
+ dev_set_drvdata(&wd_data->dev, wdd);
+ dev_set_name(&wd_data->dev, "watchdog%d", wdd->id);
+
/* Fill in the data structures */
cdev_init(&wd_data->cdev, &watchdog_fops);
- wd_data->cdev.owner = wdd->ops->owner;
/* Add the device */
- err = cdev_add(&wd_data->cdev, devno, 1);
+ err = cdev_device_add(&wd_data->cdev, &wd_data->dev);
if (err) {
pr_err("watchdog%d unable to add device %d:%d\n",
wdd->id, MAJOR(watchdog_devt), wdd->id);
if (wdd->id == 0) {
misc_deregister(&watchdog_miscdev);
old_wd_data = NULL;
- kref_put(&wd_data->kref, watchdog_core_data_release);
+ put_device(&wd_data->dev);
}
return err;
}
+ wd_data->cdev.owner = wdd->ops->owner;
+
/* Record time of most recent heartbeat as 'just before now'. */
wd_data->last_hw_keepalive = jiffies - 1;
@@ -960,7 +973,7 @@ static int watchdog_cdev_register(struct watchdog_device *wdd, dev_t devno)
*/
if (watchdog_hw_running(wdd)) {
__module_get(wdd->ops->owner);
- kref_get(&wd_data->kref);
+ get_device(&wd_data->dev);
queue_delayed_work(watchdog_wq, &wd_data->work, 0);
}
@@ -979,7 +992,7 @@ static void watchdog_cdev_unregister(struct watchdog_device *wdd)
{
struct watchdog_core_data *wd_data = wdd->wd_data;
- cdev_del(&wd_data->cdev);
+ cdev_device_del(&wd_data->cdev, &wd_data->dev);
if (wdd->id == 0) {
misc_deregister(&watchdog_miscdev);
old_wd_data = NULL;
@@ -992,15 +1005,9 @@ static void watchdog_cdev_unregister(struct watchdog_device *wdd)
cancel_delayed_work_sync(&wd_data->work);
- kref_put(&wd_data->kref, watchdog_core_data_release);
+ put_device(&wd_data->dev);
}
-static struct class watchdog_class = {
- .name = "watchdog",
- .owner = THIS_MODULE,
- .dev_groups = wdt_groups,
-};
-
/*
* watchdog_dev_register: register a watchdog device
* @wdd: watchdog device
@@ -1012,27 +1019,14 @@ static struct class watchdog_class = {
int watchdog_dev_register(struct watchdog_device *wdd)
{
- struct device *dev;
- dev_t devno;
int ret;
- devno = MKDEV(MAJOR(watchdog_devt), wdd->id);
-
- ret = watchdog_cdev_register(wdd, devno);
+ ret = watchdog_cdev_register(wdd);
if (ret)
return ret;
- dev = device_create_with_groups(&watchdog_class, wdd->parent,
- devno, wdd, wdd->groups,
- "watchdog%d", wdd->id);
- if (IS_ERR(dev)) {
- watchdog_cdev_unregister(wdd);
- return PTR_ERR(dev);
- }
-
ret = watchdog_register_pretimeout(wdd);
if (ret) {
- device_destroy(&watchdog_class, devno);
watchdog_cdev_unregister(wdd);
}
@@ -1050,7 +1044,6 @@ int watchdog_dev_register(struct watchdog_device *wdd)
void watchdog_dev_unregister(struct watchdog_device *wdd)
{
watchdog_unregister_pretimeout(wdd);
- device_destroy(&watchdog_class, wdd->wd_data->cdev.dev);
watchdog_cdev_unregister(wdd);
}
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [4.9-stable] watchdog: Fix the race between the release of watchdog_core_data and cdev
2020-05-20 20:15 ` [4.9-stable] " Ben Hutchings
@ 2020-05-21 5:45 ` Greg Kroah-Hartman
2020-05-21 5:49 ` Greg Kroah-Hartman
1 sibling, 0 replies; 4+ messages in thread
From: Greg Kroah-Hartman @ 2020-05-21 5:45 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Sasha Levin, stable
On Wed, May 20, 2020 at 09:15:57PM +0100, Ben Hutchings wrote:
> On Wed, 2020-05-20 at 21:10 +0100, Ben Hutchings wrote:
> > Please queue up the attached backport of commit 2351c88f8296 "watchdog:
> > Fix the race between the release of watchdog_core_data and cdev" for
> > 4.14.
>
> And here's the corresponding version for 4.9.
Both now queued up, thanks.
greg k-h
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [4.9-stable] watchdog: Fix the race between the release of watchdog_core_data and cdev
2020-05-20 20:15 ` [4.9-stable] " Ben Hutchings
2020-05-21 5:45 ` Greg Kroah-Hartman
@ 2020-05-21 5:49 ` Greg Kroah-Hartman
1 sibling, 0 replies; 4+ messages in thread
From: Greg Kroah-Hartman @ 2020-05-21 5:49 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Sasha Levin, stable
On Wed, May 20, 2020 at 09:15:57PM +0100, Ben Hutchings wrote:
> On Wed, 2020-05-20 at 21:10 +0100, Ben Hutchings wrote:
> > Please queue up the attached backport of commit 2351c88f8296 "watchdog:
> > Fix the race between the release of watchdog_core_data and cdev" for
> > 4.14.
>
> And here's the corresponding version for 4.9.
>
> Ben.
>
> --
> Ben Hutchings, Software Developer Codethink Ltd
> https://www.codethink.co.uk/ Dale House, 35 Dale Street
> Manchester, M1 2HF, United Kingdom
> From 1cf1b24c844a037da38e6096a865bcab75aa05eb Mon Sep 17 00:00:00 2001
> From: Kevin Hao <haokexin@gmail.com>
> Date: Tue, 8 Oct 2019 19:29:34 +0800
> Subject: watchdog: Fix the race between the release of watchdog_core_data and
> cdev
>
> commit 72139dfa2464e43957d330266994740bb7be2535 upstream.
>
> The struct cdev is embedded in the struct watchdog_core_data. In the
> current code, we manage the watchdog_core_data with a kref, but the
> cdev is manged by a kobject. There is no any relationship between
> this kref and kobject. So it is possible that the watchdog_core_data is
> freed before the cdev is entirely released. We can easily get the
> following call trace with CONFIG_DEBUG_KOBJECT_RELEASE and
> CONFIG_DEBUG_OBJECTS_TIMERS enabled.
> ODEBUG: free active (active state 0) object type: timer_list hint: delayed_work_timer_fn+0x0/0x38
> WARNING: CPU: 23 PID: 1028 at lib/debugobjects.c:481 debug_print_object+0xb0/0xf0
> Modules linked in: softdog(-) deflate ctr twofish_generic twofish_common camellia_generic serpent_generic blowfish_generic blowfish_common cast5_generic cast_common cmac xcbc af_key sch_fq_codel openvswitch nsh nf_conncount nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4
> CPU: 23 PID: 1028 Comm: modprobe Not tainted 5.3.0-next-20190924-yoctodev-standard+ #180
> Hardware name: Marvell OcteonTX CN96XX board (DT)
> pstate: 00400009 (nzcv daif +PAN -UAO)
> pc : debug_print_object+0xb0/0xf0
> lr : debug_print_object+0xb0/0xf0
> sp : ffff80001cbcfc70
> x29: ffff80001cbcfc70 x28: ffff800010ea2128
> x27: ffff800010bad000 x26: 0000000000000000
> x25: ffff80001103c640 x24: ffff80001107b268
> x23: ffff800010bad9e8 x22: ffff800010ea2128
> x21: ffff000bc2c62af8 x20: ffff80001103c600
> x19: ffff800010e867d8 x18: 0000000000000060
> x17: 0000000000000000 x16: 0000000000000000
> x15: ffff000bd7240470 x14: 6e6968207473696c
> x13: 5f72656d6974203a x12: 6570797420746365
> x11: 6a626f2029302065 x10: 7461747320657669
> x9 : 7463612820657669 x8 : 3378302f3078302b
> x7 : 0000000000001d7a x6 : ffff800010fd5889
> x5 : 0000000000000000 x4 : 0000000000000000
> x3 : 0000000000000000 x2 : ffff000bff948548
> x1 : 276a1c9e1edc2300 x0 : 0000000000000000
> Call trace:
> debug_print_object+0xb0/0xf0
> debug_check_no_obj_freed+0x1e8/0x210
> kfree+0x1b8/0x368
> watchdog_cdev_unregister+0x88/0xc8
> watchdog_dev_unregister+0x38/0x48
> watchdog_unregister_device+0xa8/0x100
> softdog_exit+0x18/0xfec4 [softdog]
> __arm64_sys_delete_module+0x174/0x200
> el0_svc_handler+0xd0/0x1c8
> el0_svc+0x8/0xc
>
> This is a common issue when using cdev embedded in a struct.
> Fortunately, we already have a mechanism to solve this kind of issue.
> Please see commit 233ed09d7fda ("chardev: add helper function to
> register char devs with a struct device") for more detail.
Wait, 233ed09d7fda ("chardev: add helper function to register char devs
with a struct device") only showed up in 4.12, it's not in 4.9, so how
is this needed for 4.9?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-05-21 5:49 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-20 20:10 [4.14-stable] watchdog: Fix the race between the release of watchdog_core_data and cdev Ben Hutchings
2020-05-20 20:15 ` [4.9-stable] " Ben Hutchings
2020-05-21 5:45 ` Greg Kroah-Hartman
2020-05-21 5:49 ` Greg Kroah-Hartman
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.