1. Patchew
  2. linux
  3. View series

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

Alexander A. Klimov posted 1 patch 4 days, 3 hours ago 
drivers/scsi/qla2xxx/qla_dfs.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
[PATCH] scsi: qla2xxx: fix NULL deref, check user input
Posted by Alexander A. Klimov 4 days, 3 hours ago 
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] staging: greybus: audio: expect 0 from kstrtoint(), not 1
Posted by Alexander A. Klimov 4 days, 3 hours ago 
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
Posted by Greg Kroah-Hartman 3 days, 13 hours ago 
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
Posted by Alexander A. Klimov 3 days, 2 hours ago 

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
Posted by Greg Kroah-Hartman 2 days, 16 hours ago 
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
Posted by Dan Carpenter 1 day, 11 hours ago 
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
Re: [PATCH] staging: greybus: audio: expect 0 from kstrtoint(), not 1
Posted by Alexander A. Klimov 2 days, 15 hours ago 

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

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.