linux-kernel - Re: [PATCH v2] thermal: core: Use strnlen in thermal_zone_device_register_with

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] [day] [month] [year] [list]

Message-ID: <20251216194413.1f12a3df@pumpkin>
Date: Tue, 16 Dec 2025 19:44:13 +0000
From: David Laight <david.laight.linux@...il.com>
To: Thorsten Blum <thorsten.blum@...ux.dev>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, Daniel Lezcano
 <daniel.lezcano@...aro.org>, Zhang Rui <rui.zhang@...el.com>, Lukasz Luba
 <lukasz.luba@....com>, linux-pm@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] thermal: core: Use strnlen in
 thermal_zone_device_register_with_trips

On Tue, 16 Dec 2025 14:09:44 +0100
Thorsten Blum <thorsten.blum@...ux.dev> wrote:

> Replace strlen() with the safer strnlen() and calculate the length of
> the thermal zone name 'type' only once.  No functional changes.
> 
> Signed-off-by: Thorsten Blum <thorsten.blum@...ux.dev>
> ---
> Changes in v2:
> - Format the code differently (Rafael)
> - Link to v1: https://lore.kernel.org/lkml/20251215121633.375193-1-thorsten.blum@linux.dev/
> ---
>  drivers/thermal/thermal_core.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 17ca5c082643..90e7edf16a52 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -1505,15 +1505,19 @@ thermal_zone_device_register_with_trips(const char *type,
>  	const struct thermal_trip *trip = trips;
>  	struct thermal_zone_device *tz;
>  	struct thermal_trip_desc *td;
> +	size_t type_len = 0;
>  	int id;
>  	int result;
>  
> -	if (!type || strlen(type) == 0) {

That one can just be:
	if (!type || !type[0])

Although one might ask 'why bother'.
Pretty much all kernel code has to assume that the callers pass reasonably
sane data.
Sanity checks for things that are easy to get wrong are one thing,
but some of the checks in this function look pretty pointless.

> +	if (type)
> +		type_len = strnlen(type, THERMAL_NAME_LENGTH);
> +
> +	if (type_len == 0) {
>  		pr_err("No thermal zone type defined\n");
>  		return ERR_PTR(-EINVAL);
>  	}
>  
> -	if (strlen(type) >= THERMAL_NAME_LENGTH) {

The code does want to check this - to stop the actual copy getting truncated
much later on - especially since at least on caller had to do an snprintf()
into a 'too long' buffer to stop gcc bleating.
But I'm not sure you need to worry about strlen() v strnlen().
(The kernel hardening people probably disagree...)

	David

> +	if (type_len == THERMAL_NAME_LENGTH) {
>  		pr_err("Thermal zone name (%s) too long, should be under %d chars\n",
>  		       type, THERMAL_NAME_LENGTH);
>  		return ERR_PTR(-EINVAL);