Re: [RFC PATCH] watchdog: HACK: disable bind attributes with NOWAYOUT

[Guenter Roeck <linux@roeck-us.net>]

On 08/28/2018 03:29 AM, Wolfram Sang wrote:
> With NOWAYOUT, prevent bind/unbind possibilities in SYSFS.
> Proof-of-concept, not for upstream yet.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
> 
> So, this is really an RFC to check if something like this is considered useful
> or not. If so, we probably need to do it differently because modifying the
> parent's driver is likely a layering violation. We could add the driver to
> modify as an optional parameter to watchdog_set_nowayout(). I wouldn't favor
> another seperate function to configure this, but am open for discussion.
> 
> Thanks,
> 
>     Wolfram
> 
>   include/linux/watchdog.h | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
> index 44985c4a1e86..241de0fa0010 100644
> --- a/include/linux/watchdog.h
> +++ b/include/linux/watchdog.h
> @@ -143,8 +143,10 @@ static inline bool watchdog_hw_running(struct watchdog_device *wdd)
>   /* Use the following function to set the nowayout feature */
>   static inline void watchdog_set_nowayout(struct watchdog_device *wdd, bool nowayout)
>   {
> -	if (nowayout)
> +	if (nowayout) {
>   		set_bit(WDOG_NO_WAY_OUT, &wdd->status);
> +		wdd->parent->driver->suppress_bind_attrs = true;

That makes sense to me. We can not assume that wdd->parent is set, so it won't work as-is.
Not sure what a "correct" solution might be. Passing "parent" as argument doesn't really
solve any layering argument - stating that it violates layering if parent is pulled from
wdd but not if it is passed as argument seems to be a bit ridiculous.

Did you ensure that the attributes are not already created by the time
suppress_bind_attrs is set ?

Thanks,
Guenter