[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

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

[Guru Das Srinagesh]

On Sun, Jun 07, 2026 at 12:23:18PM +0100, David Laight wrote:
> 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.

Sorry, could you please clarify the concern further?

`arg` is already `char *` (line 144), and I verified that my suggestion compiles fine.

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

[David Laight]

On Sun, 7 Jun 2026 12:55:13 -0700
Guru Das Srinagesh <linux@gurudas.dev> wrote:

> On Sun, Jun 07, 2026 at 12:23:18PM +0100, David Laight wrote:
> > 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.  
> 
> Sorry, could you please clarify the concern further?
> 
> `arg` is already `char *` (line 144), and I verified that my suggestion compiles fine.

But "" is 'const char *'.
Which is usually a compile error.
Unless the kernel is compiled with some permissive compilation options...

-- David

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

[Rafael J. Wysocki]

On Sat, Jun 6, 2026 at 11:05 PM Ovidiu Panait
<ovidiu.panait.oss@gmail.com> wrote:
>
> 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++) {
> --

The issue addressed by this change should be fixed by

https://lore.kernel.org/linux-pm/20260605185212.2491144-1-sam.moelius@trailofbits.com/

that has just been applied.

Thanks!

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

[Rafael J. Wysocki]

On Sat, Jun 6, 2026 at 11:05 PM Ovidiu Panait
<ovidiu.panait.oss@gmail.com> wrote:
>
> 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);
>
> --

Applied as 7.2 material along with the [2/3].

However, please note that checkpatch.pl warnings regarding the
existing code are irrelevant, so I've rewritten the changelog of this
patch to indicate the preference to use kstrtoint() rather than refer
to the warnings.

Thanks!