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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <9DA6251C-C725-46F2-899A-5CF2BE39982E@linux.dev>
Date: Sun, 9 Nov 2025 23:11:02 +0100
From: Thorsten Blum <thorsten.blum@...ux.dev>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: David Laight <david.laight.linux@...il.com>,
 Huisong Li <lihuisong@...wei.com>,
 Akira Shimahara <akira215corp@...il.com>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 stable@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] w1: therm: Fix off-by-one buffer overflow in
 alarms_store

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