I got a NULL pointer dereference report when doing fault injection test:
BUG: kernel NULL pointer dereference, address: 0000000000000009
...
RIP: 0010:static_call_del_module+0x7a/0x100
...
Call Trace:
static_call_module_notify+0x1e1/0x200
notifier_call_chain_robust+0x6f/0xe0
blocking_notifier_call_chain_robust+0x4e/0x70
load_module+0x21f7/0x2b60
__do_sys_finit_module+0xb0/0xf0
? __do_sys_finit_module+0xb0/0xf0
__x64_sys_finit_module+0x1a/0x20
do_syscall_64+0x34/0xb0
entry_SYSCALL_64_after_hwframe+0x44/0xae
When loading a module, if it fails to allocate memory for static
calls, it will delete the non-existent mods (mods == 1) in the
static_call_module_notify()'s error path.
static_call_module_notify
static_call_add_module
__static_call_init
site_mod = kzalloc() // fault injection
static_call_del_module // access non-existent mods
This patch fixes the bug by skipping the operation when the key
has no mods.
Fixes: a945c8345ec0 ("static_call: Allow early init")
Signed-off-by: Wang Hai <[email protected]>
---
kernel/static_call.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/static_call.c b/kernel/static_call.c
index 43ba0b1e0edb..c3f8ffc5a52f 100644
--- a/kernel/static_call.c
+++ b/kernel/static_call.c
@@ -400,7 +400,7 @@ static void static_call_del_module(struct module *mod)
for (site = start; site < stop; site++) {
key = static_call_key(site);
- if (key == prev_key)
+ if (key == prev_key || !static_call_key_has_mods(key))
continue;
prev_key = key;
--
2.17.1
On Sat, Oct 09, 2021 at 03:44:28PM +0800, Wang Hai wrote:
> I got a NULL pointer dereference report when doing fault injection test:
>
> BUG: kernel NULL pointer dereference, address: 0000000000000009
> ...
> RIP: 0010:static_call_del_module+0x7a/0x100
> ...
> Call Trace:
> static_call_module_notify+0x1e1/0x200
> notifier_call_chain_robust+0x6f/0xe0
> blocking_notifier_call_chain_robust+0x4e/0x70
> load_module+0x21f7/0x2b60
> __do_sys_finit_module+0xb0/0xf0
> ? __do_sys_finit_module+0xb0/0xf0
> __x64_sys_finit_module+0x1a/0x20
> do_syscall_64+0x34/0xb0
> entry_SYSCALL_64_after_hwframe+0x44/0xae
>
> When loading a module, if it fails to allocate memory for static
> calls, it will delete the non-existent mods (mods == 1) in the
> static_call_module_notify()'s error path.
>
> static_call_module_notify
> static_call_add_module
> __static_call_init
> site_mod = kzalloc() // fault injection
> static_call_del_module // access non-existent mods
>
> This patch fixes the bug by skipping the operation when the key
> has no mods.
>
> Fixes: a945c8345ec0 ("static_call: Allow early init")
> Signed-off-by: Wang Hai <[email protected]>
> ---
> kernel/static_call.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/static_call.c b/kernel/static_call.c
> index 43ba0b1e0edb..c3f8ffc5a52f 100644
> --- a/kernel/static_call.c
> +++ b/kernel/static_call.c
> @@ -400,7 +400,7 @@ static void static_call_del_module(struct module *mod)
>
> for (site = start; site < stop; site++) {
> key = static_call_key(site);
> - if (key == prev_key)
> + if (key == prev_key || !static_call_key_has_mods(key))
> continue;
>
> prev_key = key;
Should you not update prev_key in that case? Also have you looked at
jump_label_del_module() which is very similar in construction?
在 2021/10/9 18:00, Peter Zijlstra 写道:
> On Sat, Oct 09, 2021 at 03:44:28PM +0800, Wang Hai wrote:
>> I got a NULL pointer dereference report when doing fault injection test:
>>
>> BUG: kernel NULL pointer dereference, address: 0000000000000009
>> ...
>> RIP: 0010:static_call_del_module+0x7a/0x100
>> ...
>> Call Trace:
>> static_call_module_notify+0x1e1/0x200
>> notifier_call_chain_robust+0x6f/0xe0
>> blocking_notifier_call_chain_robust+0x4e/0x70
>> load_module+0x21f7/0x2b60
>> __do_sys_finit_module+0xb0/0xf0
>> ? __do_sys_finit_module+0xb0/0xf0
>> __x64_sys_finit_module+0x1a/0x20
>> do_syscall_64+0x34/0xb0
>> entry_SYSCALL_64_after_hwframe+0x44/0xae
>>
>> When loading a module, if it fails to allocate memory for static
>> calls, it will delete the non-existent mods (mods == 1) in the
>> static_call_module_notify()'s error path.
>>
>> static_call_module_notify
>> static_call_add_module
>> __static_call_init
>> site_mod = kzalloc() // fault injection
>> static_call_del_module // access non-existent mods
>>
>> This patch fixes the bug by skipping the operation when the key
>> has no mods.
>>
>> Fixes: a945c8345ec0 ("static_call: Allow early init")
>> Signed-off-by: Wang Hai <[email protected]>
>> ---
>> kernel/static_call.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/kernel/static_call.c b/kernel/static_call.c
>> index 43ba0b1e0edb..c3f8ffc5a52f 100644
>> --- a/kernel/static_call.c
>> +++ b/kernel/static_call.c
>> @@ -400,7 +400,7 @@ static void static_call_del_module(struct module *mod)
>>
>> for (site = start; site < stop; site++) {
>> key = static_call_key(site);
>> - if (key == prev_key)
>> + if (key == prev_key || !static_call_key_has_mods(key))
>> continue;
>>
>> prev_key = key;
> Should you not update prev_key in that case? Also have you looked at
> jump_label_del_module() which is very similar in construction?
> .
Thanks for your guidance, as with jump_label_del_module(),
the prev_key needs to be updated, can it be fixed like this?
--- a/kernel/static_call.c
+++ b/kernel/static_call.c
@@ -404,9 +404,9 @@ static void static_call_del_module(struct module *mod)
continue;
prev_key = key;
+ site_mod = static_call_key_next(key);
- for (prev = &key->mods, site_mod = key->mods;
- site_mod && site_mod->mod != mod;
+ for (prev = &key->mods; site_mod && site_mod->mod != mod;
prev = &site_mod->next, site_mod = site_mod->next)
;
--
Wang Hai