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]

Message-ID: <78ef0273-5ec6-f522-428a-8429f40684ca@c-s.fr>
Date:   Tue, 11 Jun 2019 07:16:37 +0200
From:   Christophe Leroy <christophe.leroy@....fr>
To:     Paul Cercueil <paul@...pouillou.net>
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 Paul,

Le 08/06/2019 à 11:57, Paul Cercueil a écrit :
> 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?

No it doesn't. This flag is for built-in functions that are needed by 
init and modules init. If the kernel doesn't support modules, it can 
drop the function after init. If the kernel supports modules, it has to 
keep the function. That's what this flag is made for.

If your need is to mark a function so that it gets discarded after init 
or module init, just mark it __init. If it is built in, it will be 
dropped after init. If it is in a module, it will be dropped after the 
module is initialised.

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

I guess it would make sense for watchdog drivers, we don't expect this 
kind of driver to be unbinded, do we ?

Christophe

> 
>> 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;
>>>
>