On 29/07/2026 19:38, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s)
> to consider:
>
> Pre-existing issues:
> - [High] Use-After-Free race condition due to dangling global pointer during
> module parameter update.
> --
>
> --- Patch [1]: [PATCH v2] params: fix charp corruption on allocation failure
> ---
> commit c59e6d7e9f9563e1394c57f9eedf07a9bb5f212b
> Author: Jiacheng Yu <[email protected]>
>
> params: fix charp corruption on allocation failure
>
> This patch fixes a corruption issue when param_set_charp() fails to allocate
> memory for the replacement string. It allocates and copies the replacement
> first, replacing the parameter value only after allocation succeeds.
>
>> diff --git a/kernel/params.c b/kernel/params.c
>> index d3a4ecab4f278..8b25133fed242 100644
>> --- a/kernel/params.c
>> +++ b/kernel/params.c
> [ ... ]
>> @@ -269,19 +270,20 @@ int param_set_charp(const char *val, const struct
>> kernel_param *kp)
>> return -ENOSPC;
>> }
>>
>> - maybe_kfree_parameter(*(char **)kp->arg);
>> -
>> /*
>> * This is a hack. We can't kmalloc() in early boot, and we
>> * don't need to; this mangled commandline is preserved.
>> */
>> if (slab_is_available()) {
>> - *(char **)kp->arg = kmalloc_parameter(len + 1);
>> - if (!*(char **)kp->arg)
>> + tmp = kmalloc_parameter(len + 1);
>> + if (!tmp)
>> return -ENOMEM;
>> - strcpy(*(char **)kp->arg, val);
>> + memcpy(tmp, val, len + 1);
>> } else
>> - *(const char **)kp->arg = val;
>> + tmp = (char *)val;
>> +
>> + maybe_kfree_parameter(*(char **)kp->arg);
>> + *(char **)kp->arg = tmp;
>
> [Severity: High]
> This isn't a bug introduced by this patch, but does this sequence leave a
> deterministic use-after-free window for concurrent readers?
>
> Because maybe_kfree_parameter() frees the old string while the global
> parameter variable still points to it, any concurrent lockless reader of
> the parameter might dereference a freed pointer before the reassignment
> on the next line.
As Petr pointed out, concurrent access to a writable charp parameter
is protected by kernel_param_lock(), as documented in
include/linux/moduleparam.h.No reorder is needed here.
>
> Could this be safely reordered to update the pointer before freeing the old
> memory? For example:
>
> char *old = *(char **)kp->arg;
> *(char **)kp->arg = tmp;
> maybe_kfree_parameter(old);
>
>>
>> return 0;
>> }
>
