History

params: fix charp corruption on allocation failure

param_set_charp() stores charp parameters in allocated memory after slab is available, and releases the previous value when the parameter is updated. The previous value is released before the replacement allocation succeeds. If kmalloc_parameter() fails, the setter returns -ENOMEM with the parameter left as NULL. Failing zswap's compressor update before zswap is initialized can later trigger: BUG: kernel NULL pointer dereference, address: 0000000000000000 RIP: 0010:strcmp+0x10/0x30 Call Trace: zswap_setup+0x3b1/0x490 zswap_enabled_param_set+0x5b/0xa0 param_attr_store+0x93/0xe0 module_attr_store+0x1c/0x30 kernfs_fop_write_iter+0x116/0x1f0 Allocate and copy the replacement first, then replace the parameter value only after allocation succeeds.

Jiacheng Yu committed 3 months ago

Commit 3dfaae01 parent 9a5ff451 file · +8 −6Browse files

Showing the changes to kernel (1 of 1 file). Show all

Unified · Split

kernel/params.c+8−6
@@ -261,6 +261,7 @@ EXPORT_SYMBOL_GPL(param_set_uint_minmax); · 260 unmodified lines
261261
262262 int param_set_charp(const char *val, const struct kernel_param *kp)
263263 {
264+ char *tmp;
264265 size_t len, maxlen = 1024;
265266
266267 len = strnlen(val, maxlen + 1);
@@ -269,19 +270,20 @@ int param_set_charp(const char *val, const struct kernel_param *kp) · 2 unmodified lines
269270 return -ENOSPC;
270271 }
271272
272− maybe_kfree_parameter(*(char **)kp->arg);
273−
274273 /*
275274 * This is a hack. We can't kmalloc() in early boot, and we
276275 * don't need to; this mangled commandline is preserved.
277276 */
278277 if (slab_is_available()) {
279− *(char **)kp->arg = kmalloc_parameter(len + 1);
280− if (!*(char **)kp->arg)
278+ tmp = kmalloc_parameter(len + 1);
279+ if (!tmp)
281280 return -ENOMEM;
282− strcpy(*(char **)kp->arg, val);
281+ memcpy(tmp, val, len + 1);
283282 } else
284− *(const char **)kp->arg = val;
283+ tmp = (char *)val;
284+
285+ maybe_kfree_parameter(*(char **)kp->arg);
286+ *(char **)kp->arg = tmp;
285287
286288 return 0;
287289 }