On 28/07/2026 20:57, Petr Pavlu wrote:
> On 7/28/26 2:22 PM, Jiacheng Yu wrote:
>> On 28/07/2026 18:46, Petr Pavlu wrote:
>>> On 7/28/26 10:55 AM, Jiacheng Yu wrote:
>>>> 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.

Agreed, memcpy() is more appropriate here since the length is already
known. Changed in v2, thanks for the review.

> 


Reply via email to