linux-kernel - Re: [PATCH v2 4/4] watchdog: jz4740: Make probe function __init_or

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 08 Jun 2019 11:57:01 +0200
From:   Paul Cercueil <paul@...pouillou.net>
To:     Christophe Leroy <christophe.leroy@....fr>
Cc:     Guenter Roeck <linux@...ck-us.net>,
        Wim Van Sebroeck <wim@...ux-watchdog.org>, od@...c.me,
        linux-watchdog@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 4/4] watchdog: jz4740: Make probe function
 __init_or_module

Hi Christophe,

Le sam. 8 juin 2019 à 9:51, Christophe Leroy <christophe.leroy@....fr> 
a écrit :
> Hi Paul,
> 
> Le 07/06/2019 à 18:24, Paul Cercueil a écrit :
>> This allows the probe function to be dropped after the kernel 
>> finished
>> its initialization, in the case where the driver was not compiled as 
>> a
>> module.
> 
> I'm not sure that's what  __init_or_module flag does.
> 
> As far as I understand, this flag makes the function being dropped 
> only when the kernel is built without modules support, ie without 
> CONFIG_MODULES. See 
> https://elixir.bootlin.com/linux/latest/source/include/linux/module.h#L145

So it doesn't depend on the driver being built-in or compiled as a 
module?

> In addition, I'm not sure you can simply define a probe function as 
> __init. What if someone tries to unbind and rebind the device through 
> sysfs for instance ?

Ouch. I feel stupid now.

> It seems there is a special function called __platform_driver_probe() 
> for registering devices when the probe function is to be in __init, 
> see 
> https://elixir.bootlin.com/linux/latest/source/drivers/base/platform.c#L684

Yes, but only usable by drivers that won't defer probe, and it removes 
the bind/unbind attributes from sysfs,
so it shouldn't be used for non-critical drivers, I think.

> Christophe
> 
>> 
>> Signed-off-by: Paul Cercueil <paul@...pouillou.net>
>> ---
>> 
>> Notes:
>>      v2: New patch
>> 
>>   drivers/watchdog/jz4740_wdt.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/watchdog/jz4740_wdt.c 
>> b/drivers/watchdog/jz4740_wdt.c
>> index 7519d80c5d05..2061788c1939 100644
>> --- a/drivers/watchdog/jz4740_wdt.c
>> +++ b/drivers/watchdog/jz4740_wdt.c
>> @@ -157,7 +157,7 @@ static const struct of_device_id 
>> jz4740_wdt_of_matches[] = {
>>   MODULE_DEVICE_TABLE(of, jz4740_wdt_of_matches);
>>   #endif
>>   -static int jz4740_wdt_probe(struct platform_device *pdev)
>> +static int __init_or_module jz4740_wdt_probe(struct platform_device 
>> *pdev)
>>   {
>>   	struct device *dev = &pdev->dev;
>>   	struct jz4740_wdt_drvdata *drvdata;
>>