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

[Thorsten Blum <thorsten.blum@linux.dev>]

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.

Thanks,
Thorsten