[PATCH 1/3] thermal: testing: Avoid NULL pointer dereference on missing arg

[PATCH 1/3] thermal: testing: Avoid NULL pointer dereference on missing arg

[Ovidiu Panait]

Commands such as deltz expect an argument after the ":" separator.
When the separator is missing, arg gets set to NULL, which is fed
directly to sscanf(). This causes a NULL ptr dereference:

$ echo deltz > /sys/kernel/debug/thermal-testing/command
BUG: kernel NULL pointer dereference, address: 0000000000000000
...
sscanf+0x57/0x80
tt_del_tz+0x39/0x1e0
tt_command_write+0x115/0x140
full_proxy_write+0x5d/0x90
vfs_write+0xd2/0x480
? srso_alias_return_thunk+0x5/0xfbef5
? count_memcg_events+0x8b/0x1a0
? srso_alias_return_thunk+0x5/0xfbef5
ksys_write+0x75/0xf0
__x64_sys_write+0x1d/0x30
x64_sys_call+0x223/0x1dd0
do_syscall_64+0x97/0x4b0
entry_SYSCALL_64_after_hwframe+0x76/0x7e

To fix this, make arg an empty string instead of leaving it NULL when the
separator is missing. sscanf() then fails correctly with -EINVAL on it.

Fixes: f6a034f2df42 ("thermal: Introduce a debugfs-based testing facility")
Signed-off-by: Ovidiu Panait <ovidiu.panait.oss@gmail.com>
---
 drivers/thermal/testing/command.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/thermal/testing/command.c b/drivers/thermal/testing/command.c
index 1159ecea57e7..5513a26feed7 100644
--- a/drivers/thermal/testing/command.c
+++ b/drivers/thermal/testing/command.c
@@ -150,6 +150,8 @@ static ssize_t tt_command_process(char *s)
 	if (arg) {
 		*arg = '\0';
 		arg++;
+	} else {
+		arg = s + strlen(s);
 	}
 
 	for (i = 0; i < ARRAY_SIZE(tt_command_strings); i++) {
-- 
2.53.0

[PATCH 2/3] thermal: testing: Replace sscanf() with kstrtoint()

[Ovidiu Panait]

Fix the following checkpatch.pl warnings:
WARNING: Prefer kstrto<type> to single variable sscanf
242: FILE: drivers/thermal/testing/zone.c:242:
+	ret = sscanf(arg, "%d", &id);

WARNING: Prefer kstrto<type> to single variable sscanf
282: FILE: drivers/thermal/testing/zone.c:282:
+	ret = sscanf(arg, "%d", &id);

Signed-off-by: Ovidiu Panait <ovidiu.panait.oss@gmail.com>
---
 drivers/thermal/testing/zone.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/thermal/testing/zone.c b/drivers/thermal/testing/zone.c
index 3c339242f52d..f7f9ca2f1f2c 100644
--- a/drivers/thermal/testing/zone.c
+++ b/drivers/thermal/testing/zone.c
@@ -239,9 +239,9 @@ int tt_del_tz(const char *arg)
 	int ret;
 	int id;
 
-	ret = sscanf(arg, "%d", &id);
-	if (ret != 1)
-		return -EINVAL;
+	ret = kstrtoint(arg, 10, &id);
+	if (ret < 0)
+		return ret;
 
 	struct tt_work *tt_work __free(kfree) = kzalloc_obj(*tt_work);
 	if (!tt_work)
@@ -279,9 +279,9 @@ static struct tt_thermal_zone *tt_get_tt_zone(const char *arg)
 	struct tt_thermal_zone *tt_zone;
 	int ret, id;
 
-	ret = sscanf(arg, "%d", &id);
-	if (ret != 1)
-		return ERR_PTR(-EINVAL);
+	ret = kstrtoint(arg, 10, &id);
+	if (ret < 0)
+		return ERR_PTR(ret);
 
 	guard(mutex)(&tt_thermal_zones_lock);
 
-- 
2.53.0

[PATCH 3/3] thermal: sysfs: Replace sscanf() with kstrtoul()

[Ovidiu Panait]

Replace sscanf() with kstrtoul() in cur_state_store(), as kstrto<type>
is preferred over single variable sscanf.

Signed-off-by: Ovidiu Panait <ovidiu.panait.oss@gmail.com>
---
 drivers/thermal/thermal_sysfs.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
index 9f2f25a6da37..b44abfc997ed 100644
--- a/drivers/thermal/thermal_sysfs.c
+++ b/drivers/thermal/thermal_sysfs.c
@@ -536,11 +536,9 @@ cur_state_store(struct device *dev, struct device_attribute *attr,
 	unsigned long state;
 	int result;
 
-	if (sscanf(buf, "%ld\n", &state) != 1)
-		return -EINVAL;
-
-	if ((long)state < 0)
-		return -EINVAL;
+	result = kstrtoul(buf, 10, &state);
+	if (result < 0)
+		return result;
 
 	/* Requested state should be less than max_state + 1 */
 	if (state > cdev->max_state)
-- 
2.53.0

Re: [PATCH 1/3] thermal: testing: Avoid NULL pointer dereference on missing arg

[Guru Das Srinagesh]

On Sun, Jun 07, 2026 at 12:04:18AM +0300, Ovidiu Panait wrote:
[...]
> To fix this, make arg an empty string instead of leaving it NULL when the
> separator is missing. sscanf() then fails correctly with -EINVAL on it.
[...]
> ---
>  drivers/thermal/testing/command.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/thermal/testing/command.c b/drivers/thermal/testing/command.c
> index 1159ecea57e7..5513a26feed7 100644
> --- a/drivers/thermal/testing/command.c
> +++ b/drivers/thermal/testing/command.c
> @@ -150,6 +150,8 @@ static ssize_t tt_command_process(char *s)
>  	if (arg) {
>  		*arg = '\0';
>  		arg++;
> +	} else {
> +		arg = s + strlen(s);
>  	}

Here, `arg` is made to point to the NUL terminator of s. Couldn't this be simplified to:

	arg = "";

to make the intent clearer? Since `tt_command_exec()` takes in arg as `const char *`,
pointing `arg` to a string literal is fine.

Re: [PATCH 1/3] thermal: testing: Avoid NULL pointer dereference on missing arg

[David Laight]

On Sat, 6 Jun 2026 19:52:49 -0700
Guru Das Srinagesh <linux@gurudas.dev> wrote:

> On Sun, Jun 07, 2026 at 12:04:18AM +0300, Ovidiu Panait wrote:
> [...]
> > To fix this, make arg an empty string instead of leaving it NULL when the
> > separator is missing. sscanf() then fails correctly with -EINVAL on it.  
> [...]
> > ---
> >  drivers/thermal/testing/command.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/thermal/testing/command.c b/drivers/thermal/testing/command.c
> > index 1159ecea57e7..5513a26feed7 100644
> > --- a/drivers/thermal/testing/command.c
> > +++ b/drivers/thermal/testing/command.c
> > @@ -150,6 +150,8 @@ static ssize_t tt_command_process(char *s)
> >  	if (arg) {
> >  		*arg = '\0';
> >  		arg++;
> > +	} else {
> > +		arg = s + strlen(s);
> >  	}  
> 
> Here, `arg` is made to point to the NUL terminator of s. Couldn't this be simplified to:
> 
> 	arg = "";
> 
> to make the intent clearer? Since `tt_command_exec()` takes in arg as `const char *`,
> pointing `arg` to a string literal is fine.
> 

Except that 'arg' itself must be 'char *' otherwise the '*arg = 0'
higher up will fail.

So you'd need to handle it differently.

-- David