[PATCH] scsi: qla2xxx: fix NULL deref, check user input

[PATCH] scsi: qla2xxx: fix NULL deref, check user input

[Alexander A. Klimov]

qla2x00_dfs_fce_write() did this:

    OUTPUT = kstrtoul(INPUT, BASE, 0);

Whenever INPUT was successfully parsed, kstrtoul() wrote its output
to *(unsigned long*)0. Otherwise, OUTPUT was set to an error value.
I added proper error handling and call kstrtoul() as expected now:

    ERROR = kstrtoul(INPUT, BASE, &OUTPUT);

Fixes: 841df27d619e ("scsi: qla2xxx: Move FCE Trace buffer allocation to user control")
Signed-off-by: Alexander A. Klimov <grandmaster@al2klimov.de>
---
 drivers/scsi/qla2xxx/qla_dfs.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/qla2xxx/qla_dfs.c b/drivers/scsi/qla2xxx/qla_dfs.c
index 43970caca7b3..efb0fb198a30 100644
--- a/drivers/scsi/qla2xxx/qla_dfs.c
+++ b/drivers/scsi/qla2xxx/qla_dfs.c
@@ -510,7 +510,14 @@ qla2x00_dfs_fce_write(struct file *file, const char __user *buffer,
 		return PTR_ERR(buf);
 	}
 
-	enable = kstrtoul(buf, 0, 0);
+	rc = kstrtoul(buf, 0, &enable);
+	if (rc) {
+		ql_dbg(ql_dbg_user, vha, 0xd03d,
+		    "fail to parse user input.");
+		rc = -EINVAL;
+		goto out_free;
+	}
+
 	rc = count;
 
 	mutex_lock(&ha->fce_mutex);
-- 
2.54.0

[PATCH] smb: smbdirect: divide, not multiply, milliseconds by 1000

[Alexander A. Klimov]

Unless smbdirect_connection_legacy_debug_proc_show()
wants to debug-log keep_alive_interval as microseconds,
a magnitude higher precision than available by the way,
keepalive_interval_msec should not be multiplied by 1000.

Fixes: a93b68d46e14 ("smb: smbdirect: introduce smbdirect_connection_legacy_debug_proc_show()")
Signed-off-by: Alexander A. Klimov <grandmaster@al2klimov.de>
---
 fs/smb/smbdirect/debug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/smb/smbdirect/debug.c b/fs/smb/smbdirect/debug.c
index 05ba7c8d165e..3445843445bf 100644
--- a/fs/smb/smbdirect/debug.c
+++ b/fs/smb/smbdirect/debug.c
@@ -40,7 +40,7 @@ void smbdirect_connection_legacy_debug_proc_show(struct smbdirect_socket *sc,
 
 	seq_puts(m, "\n");
 	seq_printf(m, "Conn keep_alive_interval: %u ",
-		   sp->keepalive_interval_msec * 1000);
+		   sp->keepalive_interval_msec / 1000);
 	seq_printf(m, "max_readwrite_size: %u rdma_readwrite_threshold: %u",
 		   sp->max_read_write_size,
 		   rdma_readwrite_threshold);
-- 
2.54.0

Re: [PATCH] smb: smbdirect: divide, not multiply, milliseconds by 1000

[Stefan Metzmacher]

Am 20.05.26 um 20:03 schrieb Alexander A. Klimov:
> Unless smbdirect_connection_legacy_debug_proc_show()
> wants to debug-log keep_alive_interval as microseconds,
> a magnitude higher precision than available by the way,
> keepalive_interval_msec should not be multiplied by 1000.
> 
> Fixes: a93b68d46e14 ("smb: smbdirect: introduce smbdirect_connection_legacy_debug_proc_show()")

We should also add
Fixes: cc55f65dd352 ("smb: client: make use of common smbdirect_socket_parameters")
as that introduced the change.

With that Reviewed-by: Stefan Metzmacher <metze@samba.org>

Thanks!
metze

> Signed-off-by: Alexander A. Klimov <grandmaster@al2klimov.de>
> ---
>   fs/smb/smbdirect/debug.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/smb/smbdirect/debug.c b/fs/smb/smbdirect/debug.c
> index 05ba7c8d165e..3445843445bf 100644
> --- a/fs/smb/smbdirect/debug.c
> +++ b/fs/smb/smbdirect/debug.c
> @@ -40,7 +40,7 @@ void smbdirect_connection_legacy_debug_proc_show(struct smbdirect_socket *sc,
>   
>   	seq_puts(m, "\n");
>   	seq_printf(m, "Conn keep_alive_interval: %u ",
> -		   sp->keepalive_interval_msec * 1000);
> +		   sp->keepalive_interval_msec / 1000);
>   	seq_printf(m, "max_readwrite_size: %u rdma_readwrite_threshold: %u",
>   		   sp->max_read_write_size,
>   		   rdma_readwrite_threshold);

Re: [PATCH] smb: smbdirect: divide, not multiply, milliseconds by 1000

[Namjae Jeon]

> diff --git a/fs/smb/smbdirect/debug.c b/fs/smb/smbdirect/debug.c
> index 05ba7c8d165e..3445843445bf 100644
> --- a/fs/smb/smbdirect/debug.c
> +++ b/fs/smb/smbdirect/debug.c
> @@ -40,7 +40,7 @@ void smbdirect_connection_legacy_debug_proc_show(struct smbdirect_socket *sc,
>
>         seq_puts(m, "\n");
>         seq_printf(m, "Conn keep_alive_interval: %u ",
           seq_printf(m, "Conn keep_alive_interval: %u sec ",

It would be nice to explicitly show the unit so readers don't have to
guess whether it's seconds or milliseconds ?
> -                  sp->keepalive_interval_msec * 1000);
> +                  sp->keepalive_interval_msec / 1000);
>         seq_printf(m, "max_readwrite_size: %u rdma_readwrite_threshold: %u",
>                    sp->max_read_write_size,
>                    rdma_readwrite_threshold);
> --
> 2.54.0
>
>

Re: [PATCH] smb: smbdirect: divide, not multiply, milliseconds by 1000

[Stefan Metzmacher]

Am 21.05.26 um 07:41 schrieb Namjae Jeon:
>> diff --git a/fs/smb/smbdirect/debug.c b/fs/smb/smbdirect/debug.c
>> index 05ba7c8d165e..3445843445bf 100644
>> --- a/fs/smb/smbdirect/debug.c
>> +++ b/fs/smb/smbdirect/debug.c
>> @@ -40,7 +40,7 @@ void smbdirect_connection_legacy_debug_proc_show(struct smbdirect_socket *sc,
>>
>>          seq_puts(m, "\n");
>>          seq_printf(m, "Conn keep_alive_interval: %u ",
>             seq_printf(m, "Conn keep_alive_interval: %u sec ",
> 
> It would be nice to explicitly show the unit so readers don't have to
> guess whether it's seconds or milliseconds ?
>> -                  sp->keepalive_interval_msec * 1000);
>> +                  sp->keepalive_interval_msec / 1000);

This all comes from keeping the output of
smbdirect_connection_legacy_debug_proc_show() stable
as it's user visible.

If we can change it anyway, I'd just output thet msec value.

>>          seq_printf(m, "max_readwrite_size: %u rdma_readwrite_threshold: %u",
>>                     sp->max_read_write_size,
>>                     rdma_readwrite_threshold);
>> --
>> 2.54.0
>>
>>

[PATCH] staging: greybus: audio: expect 0 from kstrtoint(), not 1

[Alexander A. Klimov]

kstrtoint() returns "0 on success, -ERANGE on overflow
and -EINVAL on parsing error". In contrast,
manager_sysfs_remove_store() and manager_sysfs_dump_store()
checked for 1 which always failed the operation. I fixed this.

Fixes: f9a21a3f4919 ("staging: greybus: audio_manager_sysfs: Replace sscanf with kstrto* to single variable conversion.")
Signed-off-by: Alexander A. Klimov <grandmaster@al2klimov.de>
---
 drivers/staging/greybus/audio_manager_sysfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/greybus/audio_manager_sysfs.c b/drivers/staging/greybus/audio_manager_sysfs.c
index fcd518f9540c..581791d566e3 100644
--- a/drivers/staging/greybus/audio_manager_sysfs.c
+++ b/drivers/staging/greybus/audio_manager_sysfs.c
@@ -44,7 +44,7 @@ static ssize_t manager_sysfs_remove_store(struct kobject *kobj,
 
 	int num = kstrtoint(buf, 10, &id);
 
-	if (num != 1)
+	if (num != 0)
 		return -EINVAL;
 
 	num = gb_audio_manager_remove(id);
@@ -65,7 +65,7 @@ static ssize_t manager_sysfs_dump_store(struct kobject *kobj,
 
 	int num = kstrtoint(buf, 10, &id);
 
-	if (num == 1) {
+	if (num == 0) {
 		num = gb_audio_manager_dump_module(id);
 		if (num)
 			return num;
-- 
2.54.0

Re: [PATCH] staging: greybus: audio: expect 0 from kstrtoint(), not 1

[Greg Kroah-Hartman]

On Wed, May 20, 2026 at 08:03:59PM +0200, Alexander A. Klimov wrote:
> kstrtoint() returns "0 on success, -ERANGE on overflow
> and -EINVAL on parsing error". In contrast,
> manager_sysfs_remove_store() and manager_sysfs_dump_store()
> checked for 1 which always failed the operation. I fixed this.
> 
> Fixes: f9a21a3f4919 ("staging: greybus: audio_manager_sysfs: Replace sscanf with kstrto* to single variable conversion.")
> Signed-off-by: Alexander A. Klimov <grandmaster@al2klimov.de>
> ---
>  drivers/staging/greybus/audio_manager_sysfs.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/greybus/audio_manager_sysfs.c b/drivers/staging/greybus/audio_manager_sysfs.c
> index fcd518f9540c..581791d566e3 100644
> --- a/drivers/staging/greybus/audio_manager_sysfs.c
> +++ b/drivers/staging/greybus/audio_manager_sysfs.c
> @@ -44,7 +44,7 @@ static ssize_t manager_sysfs_remove_store(struct kobject *kobj,
>  
>  	int num = kstrtoint(buf, 10, &id);
>  
> -	if (num != 1)
> +	if (num != 0)

Doesn't checkpatch now complain about this?

>  		return -EINVAL;
>  
>  	num = gb_audio_manager_remove(id);
> @@ -65,7 +65,7 @@ static ssize_t manager_sysfs_dump_store(struct kobject *kobj,
>  
>  	int num = kstrtoint(buf, 10, &id);
>  
> -	if (num == 1) {
> +	if (num == 0) {

Same here.

thanks,

greg k-h

Re: [PATCH] staging: greybus: audio: expect 0 from kstrtoint(), not 1

[Alexander A. Klimov]

On 5/21/26 10:38, Greg Kroah-Hartman wrote:
> On Wed, May 20, 2026 at 08:03:59PM +0200, Alexander A. Klimov wrote:
>> kstrtoint() returns "0 on success, -ERANGE on overflow
>> and -EINVAL on parsing error". In contrast,
>> manager_sysfs_remove_store() and manager_sysfs_dump_store()
>> checked for 1 which always failed the operation. I fixed this.
>>
>> Fixes: f9a21a3f4919 ("staging: greybus: audio_manager_sysfs: Replace sscanf with kstrto* to single variable conversion.")
>> Signed-off-by: Alexander A. Klimov <grandmaster@al2klimov.de>
>> ---
>>   drivers/staging/greybus/audio_manager_sysfs.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/greybus/audio_manager_sysfs.c b/drivers/staging/greybus/audio_manager_sysfs.c
>> index fcd518f9540c..581791d566e3 100644
>> --- a/drivers/staging/greybus/audio_manager_sysfs.c
>> +++ b/drivers/staging/greybus/audio_manager_sysfs.c
>> @@ -44,7 +44,7 @@ static ssize_t manager_sysfs_remove_store(struct kobject *kobj,
>>   
>>   	int num = kstrtoint(buf, 10, &id);
>>   
>> -	if (num != 1)
>> +	if (num != 0)
> 
> Doesn't checkpatch now complain about this?
No.

$ curl -fsSL https://lkml.org/lkml/diff/2026/5/20/2139/1 | scripts/checkpatch.pl
ERROR: Missing Signed-off-by: line(s)

total: 1 errors, 0 warnings, 0 checks, 16 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
       mechanically convert to the typical style using --fix or --fix-inplace.

Your patch has style problems, please review.

NOTE: If any of the errors are false positives, please report
       them to the maintainer, see CHECKPATCH in MAINTAINERS.
$

(Ok, sure, it says "Missing Signed-off-by:",
but https://lkml.org/lkml/diff/2026/5/20/2139/1
shows only the diff itself, not the headers.)

Re: [PATCH] staging: greybus: audio: expect 0 from kstrtoint(), not 1

[Greg Kroah-Hartman]

On Thu, May 21, 2026 at 08:42:06PM +0200, Alexander A. Klimov wrote:
> 
> 
> On 5/21/26 10:38, Greg Kroah-Hartman wrote:
> > On Wed, May 20, 2026 at 08:03:59PM +0200, Alexander A. Klimov wrote:
> > > kstrtoint() returns "0 on success, -ERANGE on overflow
> > > and -EINVAL on parsing error". In contrast,
> > > manager_sysfs_remove_store() and manager_sysfs_dump_store()
> > > checked for 1 which always failed the operation. I fixed this.
> > > 
> > > Fixes: f9a21a3f4919 ("staging: greybus: audio_manager_sysfs: Replace sscanf with kstrto* to single variable conversion.")
> > > Signed-off-by: Alexander A. Klimov <grandmaster@al2klimov.de>
> > > ---
> > >   drivers/staging/greybus/audio_manager_sysfs.c | 4 ++--
> > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/staging/greybus/audio_manager_sysfs.c b/drivers/staging/greybus/audio_manager_sysfs.c
> > > index fcd518f9540c..581791d566e3 100644
> > > --- a/drivers/staging/greybus/audio_manager_sysfs.c
> > > +++ b/drivers/staging/greybus/audio_manager_sysfs.c
> > > @@ -44,7 +44,7 @@ static ssize_t manager_sysfs_remove_store(struct kobject *kobj,
> > >   	int num = kstrtoint(buf, 10, &id);
> > > -	if (num != 1)
> > > +	if (num != 0)
> > 
> > Doesn't checkpatch now complain about this?
> No.
> 
> $ curl -fsSL https://lkml.org/lkml/diff/2026/5/20/2139/1 | scripts/checkpatch.pl

Please use lore.kernel.org, not lkml.

Also, when using b4 to apply this, it sucked in a bunch of other random
patches for you, please make new threads for everything you send.

> ERROR: Missing Signed-off-by: line(s)

Why is that showing up?

> total: 1 errors, 0 warnings, 0 checks, 16 lines checked
> 
> NOTE: For some of the reported defects, checkpatch may be able to
>       mechanically convert to the typical style using --fix or --fix-inplace.

What about trying --strict?

Anyway, think about rewriting the check for "== 0" now, that is not
normal kernel style.

thanks,

greg k-h

Re: [PATCH] staging: greybus: audio: expect 0 from kstrtoint(), not 1

[Alexander A. Klimov]

[-- Attachment #1: Type: text/plain, Size: 2767 bytes --]



On 5/22/26 07:07, Greg Kroah-Hartman wrote:
> On Thu, May 21, 2026 at 08:42:06PM +0200, Alexander A. Klimov wrote:
>>
>>
>> On 5/21/26 10:38, Greg Kroah-Hartman wrote:
>>> On Wed, May 20, 2026 at 08:03:59PM +0200, Alexander A. Klimov wrote:
>>>> kstrtoint() returns "0 on success, -ERANGE on overflow
>>>> and -EINVAL on parsing error". In contrast,
>>>> manager_sysfs_remove_store() and manager_sysfs_dump_store()
>>>> checked for 1 which always failed the operation. I fixed this.
>>>>
>>>> Fixes: f9a21a3f4919 ("staging: greybus: audio_manager_sysfs: Replace sscanf with kstrto* to single variable conversion.")
>>>> Signed-off-by: Alexander A. Klimov <grandmaster@al2klimov.de>
>>>> ---
>>>>    drivers/staging/greybus/audio_manager_sysfs.c | 4 ++--
>>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/staging/greybus/audio_manager_sysfs.c b/drivers/staging/greybus/audio_manager_sysfs.c
>>>> index fcd518f9540c..581791d566e3 100644
>>>> --- a/drivers/staging/greybus/audio_manager_sysfs.c
>>>> +++ b/drivers/staging/greybus/audio_manager_sysfs.c
>>>> @@ -44,7 +44,7 @@ static ssize_t manager_sysfs_remove_store(struct kobject *kobj,
>>>>    	int num = kstrtoint(buf, 10, &id);
>>>> -	if (num != 1)
>>>> +	if (num != 0)
>>>
>>> Doesn't checkpatch now complain about this?
>> No.
>>
>> $ curl -fsSL https://lkml.org/lkml/diff/2026/5/20/2139/1 | scripts/checkpatch.pl
> 
> Please use lore.kernel.org, not lkml.
> 
> Also, when using b4 to apply this, it sucked in a bunch of other random
> patches for you, please make new threads for everything you send.
> 
>> ERROR: Missing Signed-off-by: line(s)
> 
> Why is that showing up?

As I already said, https://lkml.org/lkml/diff/2026/5/20/2139/1
cuts off the commit message.

> 
>> total: 1 errors, 0 warnings, 0 checks, 16 lines checked
>>
>> NOTE: For some of the reported defects, checkpatch may be able to
>>        mechanically convert to the typical style using --fix or --fix-inplace.
> 
> What about trying --strict?

~/Code/linux remotes/origin/HEAD
❯ scripts/checkpatch.pl --strict 0001-*
total: 0 errors, 0 warnings, 0 checks, 16 lines checked

0001-staging-greybus-audio-expect-0-from-kstrtoint-not-1.patch has no obvious style problems and is ready for submission.

~/Code/linux remotes/origin/HEAD
❯ git log -1 --oneline
758c807bb943 (HEAD, origin/master, origin/HEAD) Merge tag 'efi-fixes-for-v7.1-2' of git://
git.kernel.org/pub/scm/linux/kernel/git/efi/efi

~/Code/linux remotes/origin/HEAD
❯

I attached 0001-* as .gz, so everyone can verify my claim.

> 
> Anyway, think about rewriting the check for "== 0" now, that is not

I'll send v2 in a minute.
I **guess** you want X,!X instead of X!=0,X==0.

> normal kernel style.

;-)
git grep -nFe '== 0)'|wc -l

[-- Attachment #2: 0001-staging-greybus-audio-expect-0-from-kstrtoint-not-1.patch.gz --]
[-- Type: application/gzip, Size: 775 bytes --]

Re: [PATCH] staging: greybus: audio: expect 0 from kstrtoint(), not 1

[Dan Carpenter]

On Fri, May 22, 2026 at 07:07:10AM +0200, Greg Kroah-Hartman wrote:
> > total: 1 errors, 0 warnings, 0 checks, 16 lines checked
> > 
> > NOTE: For some of the reported defects, checkpatch may be able to
> >       mechanically convert to the typical style using --fix or --fix-inplace.
> 
> What about trying --strict?
> 
> Anyway, think about rewriting the check for "== 0" now, that is not
> normal kernel style.

The checkpatch rules is only for NULL.  You're, of course, right that
== 0 is bad style here but there are be times where it is idiomatic to
check for == 0 or != 0.

I only bring this up to promote my blog.
https://staticthinking.wordpress.com/2024/02/20/when-to-use-0/

#SEO

regards,
dan carpenter