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

[Krzysztof Kozlowski <krzk@kernel.org>]

On 09/11/2025 23:11, Thorsten Blum wrote:
> On 9. Nov 2025, at 19:29, Krzysztof Kozlowski wrote:
>> On 30/10/2025 16:56, Thorsten Blum wrote:
>>> -	/* Convert 2nd entry to int */
>>> -	ret = kstrtoint (token, 10, &temp);
>>> -	if (ret) {
>>> -		dev_info(device,
>>> -			"%s: error parsing args %d\n", __func__, ret);
>>> -		goto free_m;
>>> +	p = endp + 1;
>>> +	temp = simple_strtol(p, &endp, 10);
>>> +	if (temp < INT_MIN || temp > INT_MAX || p == endp) {
>>> +		dev_info(device, "%s: error parsing args %d\n",
>>> +			 __func__, -EINVAL);
>>> +		goto err;
>>> 	}
>>> +	/* Cast to short to eliminate out of range values */
>>> +	th = int_to_short((int)temp);
>>>
>>> -	/* Prepare to cast to short by eliminating out of range values */
>>> -	th = int_to_short(temp);
>>> -
>>> -	/* Reorder if required th and tl */
>>> +	/* Reorder if required */
>>> 	if (tl > th)
>>> 		swap(tl, th);
>>>
>>> @@ -1897,35 +1870,30 @@ static ssize_t alarms_store(struct device *device,
>>> 	 * (th : byte 2 - tl: byte 3)
>>> 	 */
>>> 	ret = read_scratchpad(sl, &info);
>>> -	if (!ret) {
>>> -		new_config_register[0] = th;	/* Byte 2 */
>>> -		new_config_register[1] = tl;	/* Byte 3 */
>>> -		new_config_register[2] = info.rom[4];/* Byte 4 */
>>> -	} else {
>>> -		dev_info(device,
>>> -			"%s: error reading from the slave device %d\n",
>>> -			__func__, ret);
>>> -		goto free_m;
>>> +	if (ret) {
>>> +		dev_info(device, "%s: error reading from the slave device %d\n",
>>> +			 __func__, ret);
>>> +		goto err;
>>> 	}
>>> +	new_config_register[0] = th;		/* Byte 2 */
>>> +	new_config_register[1] = tl;		/* Byte 3 */
>>> +	new_config_register[2] = info.rom[4];	/* Byte 4 *
>>
>> How is this change related?
> 
> Not related, but I thought when I'm already rewriting 80% of the
> function, I might as well just improve the indentation/formatting.

Fix of buffer overflow should not contain any style changes. And
definitely changing if/else logic is just style.

Best regards,
Krzysztof