Re: [PATCH] params: fix charp corruption on allocation failure

On 7/28/26 2:22 PM, Jiacheng Yu wrote:

On 28/07/2026 18:46, Petr Pavlu wrote:

quoted

On 7/28/26 10:55 AM, Jiacheng Yu wrote:

quoted

diff --git a/kernel/params.c b/kernel/params.c
index a668863a4bb6..e4f2b71dde1e 100644
--- a/kernel/params.c
+++ b/kernel/params.c

@@ -261,6 +261,7 @@ EXPORT_SYMBOL_GPL(param_set_uint_minmax);
 
 int param_set_charp(const char *val, const struct kernel_param *kp)
 {
+	char *tmp;
 	size_t len, maxlen = 1024;
 
 	len = strnlen(val, maxlen + 1);

@@ -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);
+		strscpy(tmp, val, len + 1);

What's wrong with the plain strcpy() here?

Functionally, plain strcpy() is fine here.  The preceding
strnlen(val, maxlen + 1) either finds the NUL byte within the limit or
rejects the string, and the new allocation is exactly len + 1 bytes.

However, when this patch rewrites the line to use tmp,
checkpatch --strict reports the following warning:

    WARNING: Prefer strscpy over strcpy

Since the line is being rewritten anyway, I changed strcpy() to
strscpy() to follow that preference:
https://github.com/KSPP/linux/issues/88

I don't see a benefit to using strscpy() here. The length of the string
is already known and the tmp buffer is correctly sized, so using
`memcpy(tmp, val, len + 1)` seems most appropriate to me.

-- 
Thanks,
Petr