Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Use-after-free window in param_set_charp due to freeing old parameter before updating the pointer. -- --- Patch [1]: [PATCH] params: fix charp corruption on allocation failure --- commit 0bae951c4e6e4a0145f488c9bedd9a7a2b9a129f Author: Jiacheng Yu <[email protected]> params: fix charp corruption on allocation failure This commit fixes an issue in param_set_charp() where a failure to allocate memory for a parameter update leaves the parameter set to NULL after its old value has already been freed. It allocates and copies the new string into a temporary variable first, replacing the actual parameter only after success. > diff --git a/kernel/params.c b/kernel/params.c > --- a/kernel/params.c > +++ b/kernel/params.c [ ... ] > @@ -269,19 +270,20 @@ int param_set_charp(const char *val, const struct > kernel_param *kp) [ ... ] > 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); > + strscpy(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 is a pre-existing issue, but does freeing the old parameter before updating the pointer create a use-after-free window? By calling maybe_kfree_parameter() on the old value before assigning tmp to the global kp->arg pointer, there is a brief window where kp->arg points to freed memory. If another kernel subsystem locklessly reads this parameter during this window, could it trigger a use-after-free dereference? Since the patch rewrites this logic to use a temporary variable, would it be safer to assign tmp to *(char **)kp->arg before calling maybe_kfree_parameter() on the old string to ensure readers always see a valid pointer? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
