linux-watchdog.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH -next v2 1/2] watchdog: Clean up error handlings of __watchdog_register_device
@ 2020-11-03  6:52 Wang Wensheng
  2020-11-03  6:52 ` [PATCH -next v2 2/2] watchdog: Fix potential dereferencing of null pointer Wang Wensheng
  2020-11-03  7:05 ` [PATCH -next v2 1/2] watchdog: Clean up error handlings of __watchdog_register_device Christophe Leroy
  0 siblings, 2 replies; 4+ messages in thread
From: Wang Wensheng @ 2020-11-03  6:52 UTC (permalink / raw)
  To: wim, linux, linux-watchdog, linux-kernel; +Cc: rui.xiang, guohanjun

Signed-off-by: Wang Wensheng <wangwensheng4@huawei.com>
---
 drivers/watchdog/watchdog_core.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
index 423844757812..c73871f41142 100644
--- a/drivers/watchdog/watchdog_core.c
+++ b/drivers/watchdog/watchdog_core.c
@@ -252,10 +252,8 @@ static int __watchdog_register_device(struct watchdog_device *wdd)
 		wdd->id = id;
 
 		ret = watchdog_dev_register(wdd);
-		if (ret) {
-			ida_simple_remove(&watchdog_ida, id);
-			return ret;
-		}
+		if (ret)
+			goto id_remove;
 	}
 
 	/* Module parameter to force watchdog policy on reboot. */
@@ -273,9 +271,7 @@ static int __watchdog_register_device(struct watchdog_device *wdd)
 		if (ret) {
 			pr_err("watchdog%d: Cannot register reboot notifier (%d)\n",
 			       wdd->id, ret);
-			watchdog_dev_unregister(wdd);
-			ida_simple_remove(&watchdog_ida, id);
-			return ret;
+			goto dev_unregister;
 		}
 	}
 
@@ -289,6 +285,13 @@ static int __watchdog_register_device(struct watchdog_device *wdd)
 	}
 
 	return 0;
+
+dev_unregister:
+	watchdog_dev_unregister(wdd);
+id_remove:
+	ida_simple_remove(&watchdog_ida, id);
+
+	return ret;
 }
 
 /**
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH -next v2 2/2] watchdog: Fix potential dereferencing of null pointer
  2020-11-03  6:52 [PATCH -next v2 1/2] watchdog: Clean up error handlings of __watchdog_register_device Wang Wensheng
@ 2020-11-03  6:52 ` Wang Wensheng
  2020-11-03  7:05 ` [PATCH -next v2 1/2] watchdog: Clean up error handlings of __watchdog_register_device Christophe Leroy
  1 sibling, 0 replies; 4+ messages in thread
From: Wang Wensheng @ 2020-11-03  6:52 UTC (permalink / raw)
  To: wim, linux, linux-watchdog, linux-kernel; +Cc: rui.xiang, guohanjun

A reboot notifier, which stops the WDT by calling the stop hook without
any check, would be registered when we set WDOG_STOP_ON_REBOOT flag.

Howerer we allow the WDT driver to omit the stop hook since commit
"d0684c8a93549" ("watchdog: Make stop function optional") and provide
a module parameter for user that controls the WDOG_STOP_ON_REBOOT flag
in commit 9232c80659e94 ("watchdog: Add stop_on_reboot parameter to
control reboot policy"). Together that commits make user potential to
insert a watchdog driver that don't provide a stop hook but with the
stop_on_reboot parameter set, then dereferencing of null pointer occurs
on system reboot.

Check the stop hook before registering the reboot notifier to fix the
issue.

Signed-off-by: Wang Wensheng <wangwensheng4@huawei.com>
---
 drivers/watchdog/watchdog_core.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
index c73871f41142..5269761ba072 100644
--- a/drivers/watchdog/watchdog_core.c
+++ b/drivers/watchdog/watchdog_core.c
@@ -265,8 +265,12 @@ static int __watchdog_register_device(struct watchdog_device *wdd)
 	}
 
 	if (test_bit(WDOG_STOP_ON_REBOOT, &wdd->status)) {
-		wdd->reboot_nb.notifier_call = watchdog_reboot_notifier;
+		if (!wdd->ops->stop) {
+			ret = -EINVAL;
+			goto dev_unregister;
+		}
 
+		wdd->reboot_nb.notifier_call = watchdog_reboot_notifier;
 		ret = register_reboot_notifier(&wdd->reboot_nb);
 		if (ret) {
 			pr_err("watchdog%d: Cannot register reboot notifier (%d)\n",
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH -next v2 1/2] watchdog: Clean up error handlings of __watchdog_register_device
  2020-11-03  6:52 [PATCH -next v2 1/2] watchdog: Clean up error handlings of __watchdog_register_device Wang Wensheng
  2020-11-03  6:52 ` [PATCH -next v2 2/2] watchdog: Fix potential dereferencing of null pointer Wang Wensheng
@ 2020-11-03  7:05 ` Christophe Leroy
  2020-11-03 13:05   ` Guenter Roeck
  1 sibling, 1 reply; 4+ messages in thread
From: Christophe Leroy @ 2020-11-03  7:05 UTC (permalink / raw)
  To: Wang Wensheng, wim, linux, linux-watchdog, linux-kernel
  Cc: rui.xiang, guohanjun

Hi,

Can you provide in the commit a description of what you are doing and why ?

Christophe

Le 03/11/2020 à 07:52, Wang Wensheng a écrit :
> Signed-off-by: Wang Wensheng <wangwensheng4@huawei.com>
> ---
>   drivers/watchdog/watchdog_core.c | 17 ++++++++++-------
>   1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
> index 423844757812..c73871f41142 100644
> --- a/drivers/watchdog/watchdog_core.c
> +++ b/drivers/watchdog/watchdog_core.c
> @@ -252,10 +252,8 @@ static int __watchdog_register_device(struct watchdog_device *wdd)
>   		wdd->id = id;
>   
>   		ret = watchdog_dev_register(wdd);
> -		if (ret) {
> -			ida_simple_remove(&watchdog_ida, id);
> -			return ret;
> -		}
> +		if (ret)
> +			goto id_remove;
>   	}
>   
>   	/* Module parameter to force watchdog policy on reboot. */
> @@ -273,9 +271,7 @@ static int __watchdog_register_device(struct watchdog_device *wdd)
>   		if (ret) {
>   			pr_err("watchdog%d: Cannot register reboot notifier (%d)\n",
>   			       wdd->id, ret);
> -			watchdog_dev_unregister(wdd);
> -			ida_simple_remove(&watchdog_ida, id);
> -			return ret;
> +			goto dev_unregister;
>   		}
>   	}
>   
> @@ -289,6 +285,13 @@ static int __watchdog_register_device(struct watchdog_device *wdd)
>   	}
>   
>   	return 0;
> +
> +dev_unregister:
> +	watchdog_dev_unregister(wdd);
> +id_remove:
> +	ida_simple_remove(&watchdog_ida, id);
> +
> +	return ret;
>   }
>   
>   /**
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH -next v2 1/2] watchdog: Clean up error handlings of __watchdog_register_device
  2020-11-03  7:05 ` [PATCH -next v2 1/2] watchdog: Clean up error handlings of __watchdog_register_device Christophe Leroy
@ 2020-11-03 13:05   ` Guenter Roeck
  0 siblings, 0 replies; 4+ messages in thread
From: Guenter Roeck @ 2020-11-03 13:05 UTC (permalink / raw)
  To: Christophe Leroy, Wang Wensheng, wim, linux-watchdog, linux-kernel
  Cc: rui.xiang, guohanjun

On 11/2/20 11:05 PM, Christophe Leroy wrote:
> Hi,
> 
> Can you provide in the commit a description of what you are doing and why ?
> 

Also, I'd rather have this as second patch to make it easier (possible, actually)
to apply the first patch to -stable.

Guenter

> Christophe
> 
> Le 03/11/2020 à 07:52, Wang Wensheng a écrit :
>> Signed-off-by: Wang Wensheng <wangwensheng4@huawei.com>
>> ---
>>   drivers/watchdog/watchdog_core.c | 17 ++++++++++-------
>>   1 file changed, 10 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
>> index 423844757812..c73871f41142 100644
>> --- a/drivers/watchdog/watchdog_core.c
>> +++ b/drivers/watchdog/watchdog_core.c
>> @@ -252,10 +252,8 @@ static int __watchdog_register_device(struct watchdog_device *wdd)
>>           wdd->id = id;
>>             ret = watchdog_dev_register(wdd);
>> -        if (ret) {
>> -            ida_simple_remove(&watchdog_ida, id);
>> -            return ret;
>> -        }
>> +        if (ret)
>> +            goto id_remove;
>>       }
>>         /* Module parameter to force watchdog policy on reboot. */
>> @@ -273,9 +271,7 @@ static int __watchdog_register_device(struct watchdog_device *wdd)
>>           if (ret) {
>>               pr_err("watchdog%d: Cannot register reboot notifier (%d)\n",
>>                      wdd->id, ret);
>> -            watchdog_dev_unregister(wdd);
>> -            ida_simple_remove(&watchdog_ida, id);
>> -            return ret;
>> +            goto dev_unregister;
>>           }
>>       }
>>   @@ -289,6 +285,13 @@ static int __watchdog_register_device(struct watchdog_device *wdd)
>>       }
>>         return 0;
>> +
>> +dev_unregister:
>> +    watchdog_dev_unregister(wdd);
>> +id_remove:
>> +    ida_simple_remove(&watchdog_ida, id);
>> +
>> +    return ret;
>>   }
>>     /**
>>


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-11-03 13:05 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-03  6:52 [PATCH -next v2 1/2] watchdog: Clean up error handlings of __watchdog_register_device Wang Wensheng
2020-11-03  6:52 ` [PATCH -next v2 2/2] watchdog: Fix potential dereferencing of null pointer Wang Wensheng
2020-11-03  7:05 ` [PATCH -next v2 1/2] watchdog: Clean up error handlings of __watchdog_register_device Christophe Leroy
2020-11-03 13:05   ` Guenter Roeck

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).