[PATCH] init/main.c: check for null pointer before calling initcall

[PATCH] init/main.c: check for null pointer before calling initcall

[Abderrahmane Benbachir]

Simple check to prevent kernel panic when initcall does not exit

Signed-off-by: Abderrahmane Benbachir <abderrahmane.benbachir@polymtl.ca>
---
 init/main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/init/main.c b/init/main.c
index 0ee9c6866ada..220fd2822b61 100644
--- a/init/main.c
+++ b/init/main.c
@@ -817,6 +817,9 @@ int __init_or_module do_one_initcall(initcall_t fn)
 	int ret;
 	char msgbuf[64];
 
+	if (unlikely(!fn))
+		return -EFAULT;
+
 	if (initcall_blacklisted(fn))
 		return -EPERM;
 
-- 
2.11.0

Re: [PATCH] init/main.c: check for null pointer before calling initcall

[David Daney]

On 10/27/2017 09:47 AM, Abderrahmane Benbachir wrote:
> Simple check to prevent kernel panic when initcall does not exit

Interesting, under what circumstances do you observe the panic?

It would be best to include this information in the patch changelog.


> 
> Signed-off-by: Abderrahmane Benbachir <abderrahmane.benbachir@polymtl.ca>
> ---
>   init/main.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/init/main.c b/init/main.c
> index 0ee9c6866ada..220fd2822b61 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -817,6 +817,9 @@ int __init_or_module do_one_initcall(initcall_t fn)
>   	int ret;
>   	char msgbuf[64];
>   
> +	if (unlikely(!fn))
> +		return -EFAULT;
> +
>   	if (initcall_blacklisted(fn))
>   		return -EPERM;
>   
> 

Re: [PATCH] init/main.c: check for null pointer before calling initcall

[Thomas Gleixner]

On Fri, 27 Oct 2017, David Daney wrote:

> On 10/27/2017 09:47 AM, Abderrahmane Benbachir wrote:
> > Simple check to prevent kernel panic when initcall does not exit
> 
> Interesting, under what circumstances do you observe the panic?
> 
> It would be best to include this information in the patch changelog.

device_initcall(NULL);

might do that, but then it rightfully crashes on boot.

Re: [PATCH] init/main.c: check for null pointer before calling initcall

[David Daney]

On 10/27/2017 11:22 AM, Thomas Gleixner wrote:
> On Fri, 27 Oct 2017, David Daney wrote:
> 
>> On 10/27/2017 09:47 AM, Abderrahmane Benbachir wrote:
>>> Simple check to prevent kernel panic when initcall does not exit
>>
>> Interesting, under what circumstances do you observe the panic?
>>
>> It would be best to include this information in the patch changelog.
> 
> device_initcall(NULL);
> 
> might do that, but then it rightfully crashes on boot.
> 

That was kind of my point.  The module loader case already checks for a 
non-NULL pointer, and any NULLs in the in-kernel initializer tables 
would indicate a bigger problem that should be fixed instead.

David Daney

Re: [PATCH] init/main.c: check for null pointer before calling initcall

[Abderrahmane Benbachir]

David Daney <ddaney@caviumnetworks.com> a écrit :

> On 10/27/2017 11:22 AM, Thomas Gleixner wrote:
>> On Fri, 27 Oct 2017, David Daney wrote:
>>
>>> On 10/27/2017 09:47 AM, Abderrahmane Benbachir wrote:
>>>> Simple check to prevent kernel panic when initcall does not exit
>>>
>>> Interesting, under what circumstances do you observe the panic?
>>>
>>> It would be best to include this information in the patch changelog.
>>
>> device_initcall(NULL);
>>
>> might do that, but then it rightfully crashes on boot.
>>
>
> That was kind of my point.  The module loader case already checks  
> for a non-NULL pointer, and any NULLs in the in-kernel initializer  
> tables would indicate a bigger problem that should be fixed instead.
>
> David Daney

But this code can still be written :

static initcall_t __initcall_mymod \
    __used __section(".initcall6.init") = NULL;

In fact, I'm using the code below to assign at runtime (dynamically) which
function to be executed or not.

Re: [PATCH] init/main.c: check for null pointer before calling initcall

[David Daney]

On 10/27/2017 11:53 AM, Abderrahmane Benbachir wrote:
> 
> David Daney <ddaney@caviumnetworks.com> a écrit :
> 
>> On 10/27/2017 11:22 AM, Thomas Gleixner wrote:
>>> On Fri, 27 Oct 2017, David Daney wrote:
>>>
>>>> On 10/27/2017 09:47 AM, Abderrahmane Benbachir wrote:
>>>>> Simple check to prevent kernel panic when initcall does not exit
>>>>
>>>> Interesting, under what circumstances do you observe the panic?
>>>>
>>>> It would be best to include this information in the patch changelog.
>>>
>>> device_initcall(NULL);
>>>
>>> might do that, but then it rightfully crashes on boot.
>>>
>>
>> That was kind of my point.  The module loader case already checks for 
>> a non-NULL pointer, and any NULLs in the in-kernel initializer tables 
>> would indicate a bigger problem that should be fixed instead.
>>
>> David Daney
> 
> But this code can still be written :
> 
> static initcall_t __initcall_mymod \
>     __used __section(".initcall6.init") = NULL;
> 
> In fact, I'm using the code below to assign at runtime (dynamically) which
> function to be executed or not.
> 

Just use the standard initcall macros pointing to real functions.  If 
you want to do something tricky, put it in your own init function, and 
don't mess with the core kernel code that has been working fine for decades.

David Daney

Re: [PATCH] init/main.c: check for null pointer before calling initcall

[Thomas Gleixner]

[-- Attachment #1: Type: text/plain, Size: 1218 bytes --]

On Fri, 27 Oct 2017, Abderrahmane Benbachir wrote:
> David Daney <ddaney@caviumnetworks.com> a écrit :
> 
> > On 10/27/2017 11:22 AM, Thomas Gleixner wrote:
> > > On Fri, 27 Oct 2017, David Daney wrote:
> > > 
> > > > On 10/27/2017 09:47 AM, Abderrahmane Benbachir wrote:
> > > > > Simple check to prevent kernel panic when initcall does not exit
> > > > 
> > > > Interesting, under what circumstances do you observe the panic?
> > > > 
> > > > It would be best to include this information in the patch changelog.
> > > 
> > > device_initcall(NULL);
> > > 
> > > might do that, but then it rightfully crashes on boot.
> > > 
> > 
> > That was kind of my point.  The module loader case already checks for a
> > non-NULL pointer, and any NULLs in the in-kernel initializer tables would
> > indicate a bigger problem that should be fixed instead.
> > 
> > David Daney
> 
> But this code can still be written :
> 
> static initcall_t __initcall_mymod \
>   __used __section(".initcall6.init") = NULL;

You can write that, but it's simply wrong and abusing the initcall
system. As I said, the system rightfully crashes.

There are sane ways to do conditional initialization from within a fixed
initcall.

Thanks,

	tglx