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. 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; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
