Re: [PATCH v4] w1: therm: Fix off-by-one buffer overflow in alarms_store

[Krzysztof Kozlowski <krzk@kernel.org>]

On 16/12/2025 13:30, Thorsten Blum wrote:
> On 16. Dec 2025, at 08:11, Krzysztof Kozlowski wrote:
>> On 11/11/2025 21:44, Thorsten Blum wrote:
>>> The sysfs buffer passed to alarms_store() is allocated with 'size + 1'
>>> bytes and a NUL terminator is appended. However, the 'size' argument
>>> does not account for this extra byte. The original code then allocated
>>> 'size' bytes and used strcpy() to copy 'buf', which always writes one
>>> byte past the allocated buffer since strcpy() copies until the NUL
>>> terminator at index 'size'.
>>>
>>> Fix this by parsing the 'buf' parameter directly using simple_strtoll()
>>> without allocating any intermediate memory or string copying. This
>>> removes the overflow while simplifying the code.
>>>
>>> Cc: stable@vger.kernel.org
>>> Fixes: e2c94d6f5720 ("w1_therm: adding alarm sysfs entry")
>>> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
>>> ---
>>> [...]
>>>
>>> +	if (p == endp || *endp != ' ')
>>> +		ret = -EINVAL;
>>> +	else if (temp < INT_MIN || temp > INT_MAX)
>>> +		ret = -ERANGE;
>>> 	if (ret) {
>>> 		dev_info(device,
>>> 			"%s: error parsing args %d\n", __func__, ret);
>>> -		goto free_m;
>>> +		goto err;
>>
>> So this is just return size.
> 
> Yes, all 'goto err' could be replaced with 'return size'. I only renamed
> the label to keep the changes minimal.

You do not write commits to have minimal changes. That's not the goal.
You organize commits in logical chunks doing one thing and doing it
correctly. Empty goto label is not correct, thus should not stay.

Best regards,
Krzysztof