linux-kernel - Re: [PATCH v4] 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 for Android: free password hash cracker in your pocket

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

Message-Id: <2434C572-231F-416D-AE42-BAE8AA86B52E@linux.dev>
Date: Tue, 16 Dec 2025 13:30:01 +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 v4] w1: therm: Fix off-by-one buffer overflow in
 alarms_store

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@...r.kernel.org
>> Fixes: e2c94d6f5720 ("w1_therm: adding alarm sysfs entry")
>> Signed-off-by: Thorsten Blum <thorsten.blum@...ux.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.