diff mbox series

[smartpqi,updates,8/9] smartpqi: fix isr accessing null structure member

Message ID 20210706181618.27960-9-don.brace@microchip.com (mailing list archive)
State Superseded
Headers show
Series smartpqi updates | expand

Commit Message

Don Brace July 6, 2021, 6:16 p.m. UTC
From: Mike McGowen <mike.mcgowen@microchip.com>

Correct driver's ISR accessing a data structure member
that has not been fully initialized during driver init.
  - The pqi queue groups can be null when an interrupt fires.

Reviewed-by: Kevin Barnett <kevin.barnett@microchip.com>
Reviewed-by: Scott Benesh <scott.benesh@microchip.com>
Reviewed-by: Scott Teel <scott.teel@microchip.com>
Signed-off-by: Mike McGowen <mike.mcgowen@microchip.com>
Signed-off-by: Don Brace <don.brace@microchip.com>
---
 drivers/scsi/smartpqi/smartpqi_init.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Paul Menzel July 7, 2021, 7:48 a.m. UTC | #1
Dear Don, dear Mike,


Am 06.07.21 um 20:16 schrieb Don Brace:
> From: Mike McGowen <mike.mcgowen@microchip.com>
> 
> Correct driver's ISR accessing a data structure member
> that has not been fully initialized during driver init.

Does that crash the Linux kernel?

>    - The pqi queue groups can be null when an interrupt fires.

If it fixes a crash(?), please add a Fixes: tag so it can be backported 
to the stable series.

> Reviewed-by: Kevin Barnett <kevin.barnett@microchip.com>
> Reviewed-by: Scott Benesh <scott.benesh@microchip.com>
> Reviewed-by: Scott Teel <scott.teel@microchip.com>
> Signed-off-by: Mike McGowen <mike.mcgowen@microchip.com>
> Signed-off-by: Don Brace <don.brace@microchip.com>
> ---
>   drivers/scsi/smartpqi/smartpqi_init.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c
> index 6f2263abaa8c..eeaf0568b5e3 100644
> --- a/drivers/scsi/smartpqi/smartpqi_init.c
> +++ b/drivers/scsi/smartpqi/smartpqi_init.c
> @@ -7757,11 +7757,11 @@ static int pqi_ctrl_init(struct pqi_ctrl_info *ctrl_info)
>   
>   	pqi_init_operational_queues(ctrl_info);
>   
> -	rc = pqi_request_irqs(ctrl_info);
> +	rc = pqi_create_queues(ctrl_info);

 From a quick look, these two functions are quite different. It’d be 
great if you elaborated a bit in the commit message, what else the new 
function does.

Also, with this change, `pqi_request_irqs()` does not seem to have any 
users anymore. Without your patch:

     $ git grep pqi_request_irqs
     drivers/scsi/smartpqi/smartpqi_init.c:static int 
pqi_request_irqs(struct pqi_ctrl_info *ctrl_info)
     drivers/scsi/smartpqi/smartpqi_init.c:  rc = 
pqi_request_irqs(ctrl_info);

>   	if (rc)
>   		return rc;
>   
> -	rc = pqi_create_queues(ctrl_info);
> +	rc = pqi_request_irqs(ctrl_info);
>   	if (rc)
>   		return rc;
> 


Kind regards,

Paul
Don Brace July 8, 2021, 9:17 p.m. UTC | #2
From: Paul Menzel [mailto:pmenzel@molgen.mpg.de] 
Subject: Re: [smartpqi updates PATCH 8/9] smartpqi: fix isr accessing null structure member

Dear Don, dear Mike,


Am 06.07.21 um 20:16 schrieb Don Brace:
> From: Mike McGowen <mike.mcgowen@microchip.com>
>
> Correct driver's ISR accessing a data structure member that has not 
> been fully initialized during driver init.

Does that crash the Linux kernel?
Don: 
No. I updated the title and description to reflect this. It resulted in some brief access to uninitialized members.
This was found during some internal testing, no bugs were ever filed for this change.
 
Thanks for your review.

>    - The pqi queue groups can be null when an interrupt fires.

If it fixes a crash(?), please add a Fixes: tag so it can be backported to the stable series.



Kind regards,

Paul
diff mbox series

Patch

diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c
index 6f2263abaa8c..eeaf0568b5e3 100644
--- a/drivers/scsi/smartpqi/smartpqi_init.c
+++ b/drivers/scsi/smartpqi/smartpqi_init.c
@@ -7757,11 +7757,11 @@  static int pqi_ctrl_init(struct pqi_ctrl_info *ctrl_info)
 
 	pqi_init_operational_queues(ctrl_info);
 
-	rc = pqi_request_irqs(ctrl_info);
+	rc = pqi_create_queues(ctrl_info);
 	if (rc)
 		return rc;
 
-	rc = pqi_create_queues(ctrl_info);
+	rc = pqi_request_irqs(ctrl_info);
 	if (rc)
 		return rc;