* [PATCH] watchdog: refuse to unload softdog module when its timer is running
@ 2015-12-17 13:30 roy.qing.li
2015-12-17 14:50 ` Guenter Roeck
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: roy.qing.li @ 2015-12-17 13:30 UTC (permalink / raw)
To: linux-watchdog, wim, linux
From: Li RongQing <roy.qing.li@gmail.com>
the softdog has static variables which are accessed if its timer is
still running after the driver is unloaded. and lead to crash:
$modprobe softdog
$echo 1 >/dev/watchdog
$modprobe -r softdog
CPU 20 Unable to handle kernel paging request at virtual address
Oops[#1]:
CPU: 20 PID: 0 Comm: swapper/20 Not tainted 4.1.13-WR8.0.0.0_standard
...
Modules linked in: [last unloaded: softdog]
....
Call Trace:
[<ffffffff801e142c>] cascade+0x34/0xb0
[<ffffffff801e1964>] run_timer_softirq+0x30c/0x368
[<ffffffff80181044>] __do_softirq+0x1ec/0x418
[<ffffffff801815d0>] irq_exit+0x90/0x98
[<ffffffff8010749c>] plat_irq_dispatch+0xa4/0x140
[<ffffffff80152740>] ret_from_irq+0x0/0x4
[<ffffffff801529e0>] __r4k_wait+0x20/0x40
[<ffffffff801c2278>] cpu_startup_entry+0x2a0/0x368
[<ffffffff8015fa64>] start_secondary+0x444/0x4d8
add the module ref when timer is running to avoid to unload the softdog
module
Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
Cc: Guenter Roeck <linux@roeck-us.net>
---
drivers/watchdog/softdog.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/watchdog/softdog.c b/drivers/watchdog/softdog.c
index 0dc5e323d..86ce3f9 100644
--- a/drivers/watchdog/softdog.c
+++ b/drivers/watchdog/softdog.c
@@ -87,6 +87,7 @@ static struct timer_list watchdog_ticktock =
static void watchdog_fire(unsigned long data)
{
+ module_put(THIS_MODULE);
if (soft_noboot)
pr_crit("Triggered - Reboot ignored\n");
else if (soft_panic) {
@@ -105,13 +106,16 @@ static void watchdog_fire(unsigned long data)
static int softdog_ping(struct watchdog_device *w)
{
- mod_timer(&watchdog_ticktock, jiffies+(w->timeout*HZ));
+ if (!mod_timer(&watchdog_ticktock, jiffies+(w->timeout*HZ)))
+ __module_get(THIS_MODULE);
return 0;
}
static int softdog_stop(struct watchdog_device *w)
{
- del_timer(&watchdog_ticktock);
+ if (del_timer(&watchdog_ticktock))
+ module_put(THIS_MODULE);
+
return 0;
}
--
2.1.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] watchdog: refuse to unload softdog module when its timer is running
2015-12-17 13:30 [PATCH] watchdog: refuse to unload softdog module when its timer is running roy.qing.li
@ 2015-12-17 14:50 ` Guenter Roeck
2015-12-18 0:55 ` Li RongQing
2015-12-18 2:03 ` Guenter Roeck
2015-12-27 20:03 ` Wim Van Sebroeck
2 siblings, 1 reply; 6+ messages in thread
From: Guenter Roeck @ 2015-12-17 14:50 UTC (permalink / raw)
To: roy.qing.li, linux-watchdog, wim
On 12/17/2015 05:30 AM, roy.qing.li@gmail.com wrote:
> From: Li RongQing <roy.qing.li@gmail.com>
>
> the softdog has static variables which are accessed if its timer is
> still running after the driver is unloaded. and lead to crash:
>
> $modprobe softdog
> $echo 1 >/dev/watchdog
> $modprobe -r softdog
>
> CPU 20 Unable to handle kernel paging request at virtual address
> Oops[#1]:
> CPU: 20 PID: 0 Comm: swapper/20 Not tainted 4.1.13-WR8.0.0.0_standard
> ...
> Modules linked in: [last unloaded: softdog]
> ....
> Call Trace:
> [<ffffffff801e142c>] cascade+0x34/0xb0
> [<ffffffff801e1964>] run_timer_softirq+0x30c/0x368
> [<ffffffff80181044>] __do_softirq+0x1ec/0x418
> [<ffffffff801815d0>] irq_exit+0x90/0x98
> [<ffffffff8010749c>] plat_irq_dispatch+0xa4/0x140
> [<ffffffff80152740>] ret_from_irq+0x0/0x4
> [<ffffffff801529e0>] __r4k_wait+0x20/0x40
> [<ffffffff801c2278>] cpu_startup_entry+0x2a0/0x368
> [<ffffffff8015fa64>] start_secondary+0x444/0x4d8
>
> add the module ref when timer is running to avoid to unload the softdog
> module
>
> Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
> Cc: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/watchdog/softdog.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/watchdog/softdog.c b/drivers/watchdog/softdog.c
> index 0dc5e323d..86ce3f9 100644
> --- a/drivers/watchdog/softdog.c
> +++ b/drivers/watchdog/softdog.c
> @@ -87,6 +87,7 @@ static struct timer_list watchdog_ticktock =
>
> static void watchdog_fire(unsigned long data)
> {
> + module_put(THIS_MODULE);
Don't bother with module_put() here.
Guenter
> if (soft_noboot)
> pr_crit("Triggered - Reboot ignored\n");
> else if (soft_panic) {
> @@ -105,13 +106,16 @@ static void watchdog_fire(unsigned long data)
>
> static int softdog_ping(struct watchdog_device *w)
> {
> - mod_timer(&watchdog_ticktock, jiffies+(w->timeout*HZ));
> + if (!mod_timer(&watchdog_ticktock, jiffies+(w->timeout*HZ)))
> + __module_get(THIS_MODULE);
> return 0;
> }
>
> static int softdog_stop(struct watchdog_device *w)
> {
> - del_timer(&watchdog_ticktock);
> + if (del_timer(&watchdog_ticktock))
> + module_put(THIS_MODULE);
> +
> return 0;
> }
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] watchdog: refuse to unload softdog module when its timer is running
2015-12-17 14:50 ` Guenter Roeck
@ 2015-12-18 0:55 ` Li RongQing
2015-12-18 1:42 ` Guenter Roeck
0 siblings, 1 reply; 6+ messages in thread
From: Li RongQing @ 2015-12-18 0:55 UTC (permalink / raw)
To: Guenter Roeck; +Cc: linux-watchdog, wim
On Thu, Dec 17, 2015 at 10:50 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> Don't bother with module_put() here.
I think this is needed;
the softdog supported soft_noreboot "Softdog action, set to 1 to
ignore reboots, 0 to reboot (default=0)"
if soft_noreboot is 1, and not call module_put(THIS_MODULE) in watchdog_fire,
softdog module ref count is leaked, and softdog will not be unload
-Roy
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] watchdog: refuse to unload softdog module when its timer is running
2015-12-18 0:55 ` Li RongQing
@ 2015-12-18 1:42 ` Guenter Roeck
0 siblings, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2015-12-18 1:42 UTC (permalink / raw)
To: Li RongQing; +Cc: linux-watchdog, wim
On 12/17/2015 04:55 PM, Li RongQing wrote:
> On Thu, Dec 17, 2015 at 10:50 PM, Guenter Roeck <linux@roeck-us.net> wrote:
>> Don't bother with module_put() here.
>
>
> I think this is needed;
>
> the softdog supported soft_noreboot "Softdog action, set to 1 to
> ignore reboots, 0 to reboot (default=0)"
> if soft_noreboot is 1, and not call module_put(THIS_MODULE) in watchdog_fire,
> softdog module ref count is leaked, and softdog will not be unload
>
Good point.
Guenter
> -Roy
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] watchdog: refuse to unload softdog module when its timer is running
2015-12-17 13:30 [PATCH] watchdog: refuse to unload softdog module when its timer is running roy.qing.li
2015-12-17 14:50 ` Guenter Roeck
@ 2015-12-18 2:03 ` Guenter Roeck
2015-12-27 20:03 ` Wim Van Sebroeck
2 siblings, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2015-12-18 2:03 UTC (permalink / raw)
To: roy.qing.li, linux-watchdog, wim
On 12/17/2015 05:30 AM, roy.qing.li@gmail.com wrote:
> From: Li RongQing <roy.qing.li@gmail.com>
>
> the softdog has static variables which are accessed if its timer is
> still running after the driver is unloaded. and lead to crash:
>
> $modprobe softdog
> $echo 1 >/dev/watchdog
> $modprobe -r softdog
>
> CPU 20 Unable to handle kernel paging request at virtual address
> Oops[#1]:
> CPU: 20 PID: 0 Comm: swapper/20 Not tainted 4.1.13-WR8.0.0.0_standard
> ...
> Modules linked in: [last unloaded: softdog]
> ....
> Call Trace:
> [<ffffffff801e142c>] cascade+0x34/0xb0
> [<ffffffff801e1964>] run_timer_softirq+0x30c/0x368
> [<ffffffff80181044>] __do_softirq+0x1ec/0x418
> [<ffffffff801815d0>] irq_exit+0x90/0x98
> [<ffffffff8010749c>] plat_irq_dispatch+0xa4/0x140
> [<ffffffff80152740>] ret_from_irq+0x0/0x4
> [<ffffffff801529e0>] __r4k_wait+0x20/0x40
> [<ffffffff801c2278>] cpu_startup_entry+0x2a0/0x368
> [<ffffffff8015fa64>] start_secondary+0x444/0x4d8
>
> add the module ref when timer is running to avoid to unload the softdog
> module
>
> Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
> Cc: Guenter Roeck <linux@roeck-us.net>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/watchdog/softdog.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/watchdog/softdog.c b/drivers/watchdog/softdog.c
> index 0dc5e323d..86ce3f9 100644
> --- a/drivers/watchdog/softdog.c
> +++ b/drivers/watchdog/softdog.c
> @@ -87,6 +87,7 @@ static struct timer_list watchdog_ticktock =
>
> static void watchdog_fire(unsigned long data)
> {
> + module_put(THIS_MODULE);
> if (soft_noboot)
> pr_crit("Triggered - Reboot ignored\n");
> else if (soft_panic) {
> @@ -105,13 +106,16 @@ static void watchdog_fire(unsigned long data)
>
> static int softdog_ping(struct watchdog_device *w)
> {
> - mod_timer(&watchdog_ticktock, jiffies+(w->timeout*HZ));
> + if (!mod_timer(&watchdog_ticktock, jiffies+(w->timeout*HZ)))
> + __module_get(THIS_MODULE);
> return 0;
> }
>
> static int softdog_stop(struct watchdog_device *w)
> {
> - del_timer(&watchdog_ticktock);
> + if (del_timer(&watchdog_ticktock))
> + module_put(THIS_MODULE);
> +
> return 0;
> }
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] watchdog: refuse to unload softdog module when its timer is running
2015-12-17 13:30 [PATCH] watchdog: refuse to unload softdog module when its timer is running roy.qing.li
2015-12-17 14:50 ` Guenter Roeck
2015-12-18 2:03 ` Guenter Roeck
@ 2015-12-27 20:03 ` Wim Van Sebroeck
2 siblings, 0 replies; 6+ messages in thread
From: Wim Van Sebroeck @ 2015-12-27 20:03 UTC (permalink / raw)
To: roy.qing.li; +Cc: linux-watchdog, linux
Hi Li,
> From: Li RongQing <roy.qing.li@gmail.com>
>
> the softdog has static variables which are accessed if its timer is
> still running after the driver is unloaded. and lead to crash:
>
> $modprobe softdog
> $echo 1 >/dev/watchdog
> $modprobe -r softdog
>
> CPU 20 Unable to handle kernel paging request at virtual address
> Oops[#1]:
> CPU: 20 PID: 0 Comm: swapper/20 Not tainted 4.1.13-WR8.0.0.0_standard
> ...
> Modules linked in: [last unloaded: softdog]
> ....
> Call Trace:
> [<ffffffff801e142c>] cascade+0x34/0xb0
> [<ffffffff801e1964>] run_timer_softirq+0x30c/0x368
> [<ffffffff80181044>] __do_softirq+0x1ec/0x418
> [<ffffffff801815d0>] irq_exit+0x90/0x98
> [<ffffffff8010749c>] plat_irq_dispatch+0xa4/0x140
> [<ffffffff80152740>] ret_from_irq+0x0/0x4
> [<ffffffff801529e0>] __r4k_wait+0x20/0x40
> [<ffffffff801c2278>] cpu_startup_entry+0x2a0/0x368
> [<ffffffff8015fa64>] start_secondary+0x444/0x4d8
>
> add the module ref when timer is running to avoid to unload the softdog
> module
>
> Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
> Cc: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/watchdog/softdog.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/watchdog/softdog.c b/drivers/watchdog/softdog.c
> index 0dc5e323d..86ce3f9 100644
> --- a/drivers/watchdog/softdog.c
> +++ b/drivers/watchdog/softdog.c
> @@ -87,6 +87,7 @@ static struct timer_list watchdog_ticktock =
>
> static void watchdog_fire(unsigned long data)
> {
> + module_put(THIS_MODULE);
> if (soft_noboot)
> pr_crit("Triggered - Reboot ignored\n");
> else if (soft_panic) {
> @@ -105,13 +106,16 @@ static void watchdog_fire(unsigned long data)
>
> static int softdog_ping(struct watchdog_device *w)
> {
> - mod_timer(&watchdog_ticktock, jiffies+(w->timeout*HZ));
> + if (!mod_timer(&watchdog_ticktock, jiffies+(w->timeout*HZ)))
> + __module_get(THIS_MODULE);
> return 0;
> }
>
> static int softdog_stop(struct watchdog_device *w)
> {
> - del_timer(&watchdog_ticktock);
> + if (del_timer(&watchdog_ticktock))
> + module_put(THIS_MODULE);
> +
> return 0;
> }
>
> --
> 2.1.4
>
Patch is adde to linux-watchdog-next.
Kind regards,
Wim.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-12-27 20:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-12-17 13:30 [PATCH] watchdog: refuse to unload softdog module when its timer is running roy.qing.li
2015-12-17 14:50 ` Guenter Roeck
2015-12-18 0:55 ` Li RongQing
2015-12-18 1:42 ` Guenter Roeck
2015-12-18 2:03 ` Guenter Roeck
2015-12-27 20:03 ` Wim Van Sebroeck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).