Skip to contentSkip to conversation

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

PATCH In review Abderrahmane Benbachir 3 participants 7 messages Oct 27, 2017

lore
Share & tools
lore
#0 lore raw

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

Simple check to prevent kernel panic when initcall does not exit

---
 init/main.c | 3 +++
 1 file changed, 3 insertions(+)
init/main.c +3 -0
index 0ee9c6866ada..220fd2822b61 100644
@@ -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;
 
Signature · 1 line
2.11.0
#1 lore raw
Quoted 1 line · Abderrahmane Benbachir

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.

Quoted 21 lines
---
  init/main.c | 3 +++
  1 file changed, 3 insertions(+)
init/main.c +3 -0
index 0ee9c6866ada..220fd2822b61 100644
@@ -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 lore raw
Quoted 6 lines · David Daney
Quoted 1 line · Abderrahmane Benbachir

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.

#3 lore raw
Quoted 5 lines · Thomas Gleixner
Quoted 3 lines · David Daney
Quoted 1 line · Abderrahmane Benbachir

Simple check to prevent kernel panic when initcall does not exit

Quoted 8 lines
Quoted 3 lines

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

#4 lore raw

David Daney <[email protected]> a écrit :

Quoted 10 lines · Thomas Gleixner
Quoted 9 lines · David Daney
Quoted 7 lines · Abderrahmane Benbachir
Quoted 1 line

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.

Quoted 8 lines
Quoted 4 lines

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.

#5 lore raw
Quoted 22 lines · Abderrahmane Benbachir

David Daney <[email protected]> a écrit :

Quoted 10 lines · Thomas Gleixner
Quoted 9 lines · David Daney
Quoted 7 lines · Abderrahmane Benbachir
Quoted 1 line

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.

Quoted 8 lines
Quoted 4 lines

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

Quoted 8 lines

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

#6 lore raw
In reply to Abderrahmane Benbachir
Quoted 21 lines · Abderrahmane Benbachir

David Daney <[email protected]> a écrit :

Quoted 19 lines · Thomas Gleixner
Quoted 5 lines · David Daney
Quoted 4 lines · Abderrahmane Benbachir
Quoted 1 line

Simple check to prevent kernel panic when initcall does not exit

Quoted 8 lines
Quoted 3 lines

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.

Quoted 6 lines
Quoted 1 line

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