linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] scsi: mpt3sas: Fix NULL pointer crash due to missing check device hostdata
@ 2022-08-25  9:26 huhai
  2022-09-01  1:11 ` huhai
  0 siblings, 1 reply; 5+ messages in thread
From: huhai @ 2022-08-25  9:26 UTC (permalink / raw)
  To: jejb, martin.petersen
  Cc: sathya.prakash, sreekanth.reddy, suganath-prabu.subramani,
	MPT-FusionLinux.pdl, linux-scsi, linux-kernel, huhai, stable,
	Jackie Liu

From: huhai <huhai@kylinos.cn>

If _scsih_io_done() is called with scmd->device->hostdata=NULL, it can lead
to the following panic:

  BUG: unable to handle kernel NULL pointer dereference at 0000000000000018
  PGD 4547a4067 P4D 4547a4067 PUD 0
  Oops: 0002 [#1] SMP NOPTI
  CPU: 62 PID: 0 Comm: swapper/62 Kdump: loaded Not tainted 4.19.90-24.4.v2101.ky10.x86_64 #1
  Hardware name: Storage Server/65N32-US, BIOS SQL1041217 05/30/2022
  RIP: 0010:_scsih_set_satl_pending+0x2d/0x50 [mpt3sas]
  Code: 00 00 48 8b 87 60 01 00 00 0f b6 10 80 fa a1 74 09 31 c0 80 fa 85 74 02 f3 c3 48 8b 47 38 40 84 f6 48 8b 80 98 00 00 00 75 08 <f0> 80 60 18 fe 31 c0 c3 f0 48 0f ba 68 18 00 0f 92 c0 0f b6 c0 c3
  RSP: 0018:ffff8ec22fc03e00 EFLAGS: 00010046
  RAX: 0000000000000000 RBX: ffff8eba1b072518 RCX: 0000000000000001
  RDX: 0000000000000085 RSI: 0000000000000000 RDI: ffff8eba1b072518
  RBP: 0000000000000dbd R08: 0000000000000000 R09: 0000000000029700
  R10: ffff8ec22fc03f80 R11: 0000000000000000 R12: ffff8ebe2d3609e8
  R13: ffff8ebe2a72b600 R14: ffff8eca472707e0 R15: 0000000000000020
  FS:  0000000000000000(0000) GS:ffff8ec22fc00000(0000) knlGS:0000000000000000
  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
  CR2: 0000000000000018 CR3: 000000046e5f6000 CR4: 00000000003406e0
  Call Trace:
   <IRQ>
   _scsih_io_done+0x4a/0x9f0 [mpt3sas]
   _base_interrupt+0x23f/0xe10 [mpt3sas]
   __handle_irq_event_percpu+0x40/0x190
   handle_irq_event_percpu+0x30/0x70
   handle_irq_event+0x36/0x60
   handle_edge_irq+0x7e/0x190
   handle_irq+0xa8/0x110
   do_IRQ+0x49/0xe0

Fix it by move scmd->device->hostdata check before _scsih_set_satl_pending
called.

Other changes:
- It looks clear to move get mpi_reply to near its check.

Fixes: ffb584565894 ("scsi: mpt3sas: fix hang on ata passthrough commands")
Cc: <stable@vger.kernel.org> # v4.9+
Co-developed-by: Jackie Liu <liuyun01@kylinos.cn>
Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
Signed-off-by: huhai <huhai@kylinos.cn>
---
 drivers/scsi/mpt3sas/mpt3sas_scsih.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index def37a7e5980..85f5749a0421 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -5704,27 +5704,26 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16 smid, u8 msix_index, u32 reply)
 	struct MPT3SAS_DEVICE *sas_device_priv_data;
 	u32 response_code = 0;
 
-	mpi_reply = mpt3sas_base_get_reply_virt_addr(ioc, reply);
-
 	scmd = mpt3sas_scsih_scsi_lookup_get(ioc, smid);
 	if (scmd == NULL)
 		return 1;
 
+	sas_device_priv_data = scmd->device->hostdata;
+	if (!sas_device_priv_data || !sas_device_priv_data->sas_target ||
+	     sas_device_priv_data->sas_target->deleted) {
+		scmd->result = DID_NO_CONNECT << 16;
+		goto out;
+	}
 	_scsih_set_satl_pending(scmd, false);
 
 	mpi_request = mpt3sas_base_get_msg_frame(ioc, smid);
 
+	mpi_reply = mpt3sas_base_get_reply_virt_addr(ioc, reply);
 	if (mpi_reply == NULL) {
 		scmd->result = DID_OK << 16;
 		goto out;
 	}
 
-	sas_device_priv_data = scmd->device->hostdata;
-	if (!sas_device_priv_data || !sas_device_priv_data->sas_target ||
-	     sas_device_priv_data->sas_target->deleted) {
-		scmd->result = DID_NO_CONNECT << 16;
-		goto out;
-	}
 	ioc_status = le16_to_cpu(mpi_reply->IOCStatus);
 
 	/*
-- 
2.27.0


No virus found
		Checked by Hillstone Network AntiVirus


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re:[PATCH] scsi: mpt3sas: Fix NULL pointer crash due to missing check device hostdata
  2022-08-25  9:26 [PATCH] scsi: mpt3sas: Fix NULL pointer crash due to missing check device hostdata huhai
@ 2022-09-01  1:11 ` huhai
  2022-09-01  5:08   ` [PATCH] " Sathya Prakash Veerichetty
  0 siblings, 1 reply; 5+ messages in thread
From: huhai @ 2022-09-01  1:11 UTC (permalink / raw)
  To: jejb, martin.petersen
  Cc: sathya.prakash, sreekanth.reddy, suganath-prabu.subramani,
	MPT-FusionLinux.pdl, linux-scsi, linux-kernel, huhai, stable,
	Jackie Liu

Friendly ping.


At 2022-08-25 17:26:45, "huhai" <15815827059@163.com> wrote:
>From: huhai <huhai@kylinos.cn>
>
>If _scsih_io_done() is called with scmd->device->hostdata=NULL, it can lead
>to the following panic:
>
>  BUG: unable to handle kernel NULL pointer dereference at 0000000000000018
>  PGD 4547a4067 P4D 4547a4067 PUD 0
>  Oops: 0002 [#1] SMP NOPTI
>  CPU: 62 PID: 0 Comm: swapper/62 Kdump: loaded Not tainted 4.19.90-24.4.v2101.ky10.x86_64 #1
>  Hardware name: Storage Server/65N32-US, BIOS SQL1041217 05/30/2022
>  RIP: 0010:_scsih_set_satl_pending+0x2d/0x50 [mpt3sas]
>  Code: 00 00 48 8b 87 60 01 00 00 0f b6 10 80 fa a1 74 09 31 c0 80 fa 85 74 02 f3 c3 48 8b 47 38 40 84 f6 48 8b 80 98 00 00 00 75 08 <f0> 80 60 18 fe 31 c0 c3 f0 48 0f ba 68 18 00 0f 92 c0 0f b6 c0 c3
>  RSP: 0018:ffff8ec22fc03e00 EFLAGS: 00010046
>  RAX: 0000000000000000 RBX: ffff8eba1b072518 RCX: 0000000000000001
>  RDX: 0000000000000085 RSI: 0000000000000000 RDI: ffff8eba1b072518
>  RBP: 0000000000000dbd R08: 0000000000000000 R09: 0000000000029700
>  R10: ffff8ec22fc03f80 R11: 0000000000000000 R12: ffff8ebe2d3609e8
>  R13: ffff8ebe2a72b600 R14: ffff8eca472707e0 R15: 0000000000000020
>  FS:  0000000000000000(0000) GS:ffff8ec22fc00000(0000) knlGS:0000000000000000
>  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>  CR2: 0000000000000018 CR3: 000000046e5f6000 CR4: 00000000003406e0
>  Call Trace:
>   <IRQ>
>   _scsih_io_done+0x4a/0x9f0 [mpt3sas]
>   _base_interrupt+0x23f/0xe10 [mpt3sas]
>   __handle_irq_event_percpu+0x40/0x190
>   handle_irq_event_percpu+0x30/0x70
>   handle_irq_event+0x36/0x60
>   handle_edge_irq+0x7e/0x190
>   handle_irq+0xa8/0x110
>   do_IRQ+0x49/0xe0
>
>Fix it by move scmd->device->hostdata check before _scsih_set_satl_pending
>called.
>
>Other changes:
>- It looks clear to move get mpi_reply to near its check.
>
>Fixes: ffb584565894 ("scsi: mpt3sas: fix hang on ata passthrough commands")
>Cc: <stable@vger.kernel.org> # v4.9+
>Co-developed-by: Jackie Liu <liuyun01@kylinos.cn>
>Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
>Signed-off-by: huhai <huhai@kylinos.cn>
>---
> drivers/scsi/mpt3sas/mpt3sas_scsih.c | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
>diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
>index def37a7e5980..85f5749a0421 100644
>--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
>+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
>@@ -5704,27 +5704,26 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16 smid, u8 msix_index, u32 reply)
> 	struct MPT3SAS_DEVICE *sas_device_priv_data;
> 	u32 response_code = 0;
> 
>-	mpi_reply = mpt3sas_base_get_reply_virt_addr(ioc, reply);
>-
> 	scmd = mpt3sas_scsih_scsi_lookup_get(ioc, smid);
> 	if (scmd == NULL)
> 		return 1;
> 
>+	sas_device_priv_data = scmd->device->hostdata;
>+	if (!sas_device_priv_data || !sas_device_priv_data->sas_target ||
>+	     sas_device_priv_data->sas_target->deleted) {
>+		scmd->result = DID_NO_CONNECT << 16;
>+		goto out;
>+	}
> 	_scsih_set_satl_pending(scmd, false);
> 
> 	mpi_request = mpt3sas_base_get_msg_frame(ioc, smid);
> 
>+	mpi_reply = mpt3sas_base_get_reply_virt_addr(ioc, reply);
> 	if (mpi_reply == NULL) {
> 		scmd->result = DID_OK << 16;
> 		goto out;
> 	}
> 
>-	sas_device_priv_data = scmd->device->hostdata;
>-	if (!sas_device_priv_data || !sas_device_priv_data->sas_target ||
>-	     sas_device_priv_data->sas_target->deleted) {
>-		scmd->result = DID_NO_CONNECT << 16;
>-		goto out;
>-	}
> 	ioc_status = le16_to_cpu(mpi_reply->IOCStatus);
> 
> 	/*
>-- 
>2.27.0
>
>
>No virus found
>		Checked by Hillstone Network AntiVirus

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] scsi: mpt3sas: Fix NULL pointer crash due to missing check device hostdata
  2022-09-01  1:11 ` huhai
@ 2022-09-01  5:08   ` Sathya Prakash Veerichetty
  2022-09-01  6:03     ` huhai
  0 siblings, 1 reply; 5+ messages in thread
From: Sathya Prakash Veerichetty @ 2022-09-01  5:08 UTC (permalink / raw)
  To: huhai
  Cc: jejb, Martin K. Petersen, Sreekanth Reddy,
	Suganath Prabu Subramani, PDL-MPT-FUSIONLINUX, linux-scsi,
	linux-kernel, huhai, stable, Jackie Liu

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

The patch could be improved to clear the ata_cmd_pending bit for the
cases !sas_device_priv_data->sas_target and
sas_device_priv_data->sas_target->deleted before returning
DID_NO_CONNECT to retain the current functionality.


On Wed, Aug 31, 2022 at 7:11 PM huhai <15815827059@163.com> wrote:
>
> Friendly ping.
>
>
> At 2022-08-25 17:26:45, "huhai" <15815827059@163.com> wrote:
> >From: huhai <huhai@kylinos.cn>
> >
> >If _scsih_io_done() is called with scmd->device->hostdata=NULL, it can lead
> >to the following panic:
> >
> >  BUG: unable to handle kernel NULL pointer dereference at 0000000000000018
> >  PGD 4547a4067 P4D 4547a4067 PUD 0
> >  Oops: 0002 [#1] SMP NOPTI
> >  CPU: 62 PID: 0 Comm: swapper/62 Kdump: loaded Not tainted 4.19.90-24.4.v2101.ky10.x86_64 #1
> >  Hardware name: Storage Server/65N32-US, BIOS SQL1041217 05/30/2022
> >  RIP: 0010:_scsih_set_satl_pending+0x2d/0x50 [mpt3sas]
> >  Code: 00 00 48 8b 87 60 01 00 00 0f b6 10 80 fa a1 74 09 31 c0 80 fa 85 74 02 f3 c3 48 8b 47 38 40 84 f6 48 8b 80 98 00 00 00 75 08 <f0> 80 60 18 fe 31 c0 c3 f0 48 0f ba 68 18 00 0f 92 c0 0f b6 c0 c3
> >  RSP: 0018:ffff8ec22fc03e00 EFLAGS: 00010046
> >  RAX: 0000000000000000 RBX: ffff8eba1b072518 RCX: 0000000000000001
> >  RDX: 0000000000000085 RSI: 0000000000000000 RDI: ffff8eba1b072518
> >  RBP: 0000000000000dbd R08: 0000000000000000 R09: 0000000000029700
> >  R10: ffff8ec22fc03f80 R11: 0000000000000000 R12: ffff8ebe2d3609e8
> >  R13: ffff8ebe2a72b600 R14: ffff8eca472707e0 R15: 0000000000000020
> >  FS:  0000000000000000(0000) GS:ffff8ec22fc00000(0000) knlGS:0000000000000000
> >  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> >  CR2: 0000000000000018 CR3: 000000046e5f6000 CR4: 00000000003406e0
> >  Call Trace:
> >   <IRQ>
> >   _scsih_io_done+0x4a/0x9f0 [mpt3sas]
> >   _base_interrupt+0x23f/0xe10 [mpt3sas]
> >   __handle_irq_event_percpu+0x40/0x190
> >   handle_irq_event_percpu+0x30/0x70
> >   handle_irq_event+0x36/0x60
> >   handle_edge_irq+0x7e/0x190
> >   handle_irq+0xa8/0x110
> >   do_IRQ+0x49/0xe0
> >
> >Fix it by move scmd->device->hostdata check before _scsih_set_satl_pending
> >called.
> >
> >Other changes:
> >- It looks clear to move get mpi_reply to near its check.
> >
> >Fixes: ffb584565894 ("scsi: mpt3sas: fix hang on ata passthrough commands")
> >Cc: <stable@vger.kernel.org> # v4.9+
> >Co-developed-by: Jackie Liu <liuyun01@kylinos.cn>
> >Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
> >Signed-off-by: huhai <huhai@kylinos.cn>
> >---
> > drivers/scsi/mpt3sas/mpt3sas_scsih.c | 15 +++++++--------
> > 1 file changed, 7 insertions(+), 8 deletions(-)
> >
> >diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> >index def37a7e5980..85f5749a0421 100644
> >--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> >+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> >@@ -5704,27 +5704,26 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16 smid, u8 msix_index, u32 reply)
> >       struct MPT3SAS_DEVICE *sas_device_priv_data;
> >       u32 response_code = 0;
> >
> >-      mpi_reply = mpt3sas_base_get_reply_virt_addr(ioc, reply);
> >-
> >       scmd = mpt3sas_scsih_scsi_lookup_get(ioc, smid);
> >       if (scmd == NULL)
> >               return 1;
> >
> >+      sas_device_priv_data = scmd->device->hostdata;
> >+      if (!sas_device_priv_data || !sas_device_priv_data->sas_target ||
> >+           sas_device_priv_data->sas_target->deleted) {
> >+              scmd->result = DID_NO_CONNECT << 16;
> >+              goto out;
> >+      }
> >       _scsih_set_satl_pending(scmd, false);
> >
> >       mpi_request = mpt3sas_base_get_msg_frame(ioc, smid);
> >
> >+      mpi_reply = mpt3sas_base_get_reply_virt_addr(ioc, reply);
> >       if (mpi_reply == NULL) {
> >               scmd->result = DID_OK << 16;
> >               goto out;
> >       }
> >
> >-      sas_device_priv_data = scmd->device->hostdata;
> >-      if (!sas_device_priv_data || !sas_device_priv_data->sas_target ||
> >-           sas_device_priv_data->sas_target->deleted) {
> >-              scmd->result = DID_NO_CONNECT << 16;
> >-              goto out;
> >-      }
> >       ioc_status = le16_to_cpu(mpi_reply->IOCStatus);
> >
> >       /*
> >--
> >2.27.0
> >
> >
> >No virus found
> >               Checked by Hillstone Network AntiVirus

-- 
This electronic communication and the information and any files transmitted 
with it, or attached to it, are confidential and are intended solely for 
the use of the individual or entity to whom it is addressed and may contain 
information that is confidential, legally privileged, protected by privacy 
laws, or otherwise restricted from disclosure to anyone else. If you are 
not the intended recipient or the person responsible for delivering the 
e-mail to the intended recipient, you are hereby notified that any use, 
copying, distributing, dissemination, forwarding, printing, or copying of 
this e-mail is strictly prohibited. If you received this e-mail in error, 
please return the e-mail to the sender, delete it from your computer, and 
destroy any printed copy of it.

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4227 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re:Re: [PATCH] scsi: mpt3sas: Fix NULL pointer crash due to missing check device hostdata
  2022-09-01  5:08   ` [PATCH] " Sathya Prakash Veerichetty
@ 2022-09-01  6:03     ` huhai
  2022-09-01  6:12       ` Jackie Liu
  0 siblings, 1 reply; 5+ messages in thread
From: huhai @ 2022-09-01  6:03 UTC (permalink / raw)
  To: Sathya Prakash Veerichetty
  Cc: jejb, Martin K. Petersen, Sreekanth Reddy,
	Suganath Prabu Subramani, PDL-MPT-FUSIONLINUX, linux-scsi,
	linux-kernel, huhai, stable, Jackie Liu


At 2022-09-01 13:08:14, "Sathya Prakash Veerichetty" <sathya.prakash@broadcom.com> wrote:
>The patch could be improved to clear the ata_cmd_pending bit for the
>cases !sas_device_priv_data->sas_target and
>sas_device_priv_data->sas_target->deleted before returning

>DID_NO_CONNECT to retain the current functionality.

Hi, 

Maybe my commit information is not clear enough,This patch is fixed NULL pointer crash
duto to "struct MPT3SAS_DEVICE *priv = scmd->device->hostdata;"  got a NULL pointer, and
when "clear_bit(0, &priv->ata_command_pending);" is called, kernel will panic. 

Thanks

>
>
>On Wed, Aug 31, 2022 at 7:11 PM huhai <15815827059@163.com> wrote:
>>
>> Friendly ping.
>>
>>
>> At 2022-08-25 17:26:45, "huhai" <15815827059@163.com> wrote:
>> >From: huhai <huhai@kylinos.cn>
>> >
>> >If _scsih_io_done() is called with scmd->device->hostdata=NULL, it can lead
>> >to the following panic:
>> >
>> >  BUG: unable to handle kernel NULL pointer dereference at 0000000000000018
>> >  PGD 4547a4067 P4D 4547a4067 PUD 0
>> >  Oops: 0002 [#1] SMP NOPTI
>> >  CPU: 62 PID: 0 Comm: swapper/62 Kdump: loaded Not tainted 4.19.90-24.4.v2101.ky10.x86_64 #1
>> >  Hardware name: Storage Server/65N32-US, BIOS SQL1041217 05/30/2022
>> >  RIP: 0010:_scsih_set_satl_pending+0x2d/0x50 [mpt3sas]
>> >  Code: 00 00 48 8b 87 60 01 00 00 0f b6 10 80 fa a1 74 09 31 c0 80 fa 85 74 02 f3 c3 48 8b 47 38 40 84 f6 48 8b 80 98 00 00 00 75 08 <f0> 80 60 18 fe 31 c0 c3 f0 48 0f ba 68 18 00 0f 92 c0 0f b6 c0 c3
>> >  RSP: 0018:ffff8ec22fc03e00 EFLAGS: 00010046
>> >  RAX: 0000000000000000 RBX: ffff8eba1b072518 RCX: 0000000000000001
>> >  RDX: 0000000000000085 RSI: 0000000000000000 RDI: ffff8eba1b072518
>> >  RBP: 0000000000000dbd R08: 0000000000000000 R09: 0000000000029700
>> >  R10: ffff8ec22fc03f80 R11: 0000000000000000 R12: ffff8ebe2d3609e8
>> >  R13: ffff8ebe2a72b600 R14: ffff8eca472707e0 R15: 0000000000000020
>> >  FS:  0000000000000000(0000) GS:ffff8ec22fc00000(0000) knlGS:0000000000000000
>> >  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> >  CR2: 0000000000000018 CR3: 000000046e5f6000 CR4: 00000000003406e0
>> >  Call Trace:
>> >   <IRQ>
>> >   _scsih_io_done+0x4a/0x9f0 [mpt3sas]
>> >   _base_interrupt+0x23f/0xe10 [mpt3sas]
>> >   __handle_irq_event_percpu+0x40/0x190
>> >   handle_irq_event_percpu+0x30/0x70
>> >   handle_irq_event+0x36/0x60
>> >   handle_edge_irq+0x7e/0x190
>> >   handle_irq+0xa8/0x110
>> >   do_IRQ+0x49/0xe0
>> >
>> >Fix it by move scmd->device->hostdata check before _scsih_set_satl_pending
>> >called.
>> >
>> >Other changes:
>> >- It looks clear to move get mpi_reply to near its check.
>> >
>> >Fixes: ffb584565894 ("scsi: mpt3sas: fix hang on ata passthrough commands")
>> >Cc: <stable@vger.kernel.org> # v4.9+
>> >Co-developed-by: Jackie Liu <liuyun01@kylinos.cn>
>> >Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
>> >Signed-off-by: huhai <huhai@kylinos.cn>
>> >---
>> > drivers/scsi/mpt3sas/mpt3sas_scsih.c | 15 +++++++--------
>> > 1 file changed, 7 insertions(+), 8 deletions(-)
>> >
>> >diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
>> >index def37a7e5980..85f5749a0421 100644
>> >--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
>> >+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
>> >@@ -5704,27 +5704,26 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16 smid, u8 msix_index, u32 reply)
>> >       struct MPT3SAS_DEVICE *sas_device_priv_data;
>> >       u32 response_code = 0;
>> >
>> >-      mpi_reply = mpt3sas_base_get_reply_virt_addr(ioc, reply);
>> >-
>> >       scmd = mpt3sas_scsih_scsi_lookup_get(ioc, smid);
>> >       if (scmd == NULL)
>> >               return 1;
>> >
>> >+      sas_device_priv_data = scmd->device->hostdata;
>> >+      if (!sas_device_priv_data || !sas_device_priv_data->sas_target ||
>> >+           sas_device_priv_data->sas_target->deleted) {
>> >+              scmd->result = DID_NO_CONNECT << 16;
>> >+              goto out;
>> >+      }
>> >       _scsih_set_satl_pending(scmd, false);
>> >
>> >       mpi_request = mpt3sas_base_get_msg_frame(ioc, smid);
>> >
>> >+      mpi_reply = mpt3sas_base_get_reply_virt_addr(ioc, reply);
>> >       if (mpi_reply == NULL) {
>> >               scmd->result = DID_OK << 16;
>> >               goto out;
>> >       }
>> >
>> >-      sas_device_priv_data = scmd->device->hostdata;
>> >-      if (!sas_device_priv_data || !sas_device_priv_data->sas_target ||
>> >-           sas_device_priv_data->sas_target->deleted) {
>> >-              scmd->result = DID_NO_CONNECT << 16;
>> >-              goto out;
>> >-      }
>> >       ioc_status = le16_to_cpu(mpi_reply->IOCStatus);
>> >
>> >       /*
>> >--
>> >2.27.0
>> >
>> >
>> >No virus found
>> >               Checked by Hillstone Network AntiVirus
>
>-- 
>This electronic communication and the information and any files transmitted 
>with it, or attached to it, are confidential and are intended solely for 
>the use of the individual or entity to whom it is addressed and may contain 
>information that is confidential, legally privileged, protected by privacy 
>laws, or otherwise restricted from disclosure to anyone else. If you are 
>not the intended recipient or the person responsible for delivering the 
>e-mail to the intended recipient, you are hereby notified that any use, 
>copying, distributing, dissemination, forwarding, printing, or copying of 
>this e-mail is strictly prohibited. If you received this e-mail in error, 
>please return the e-mail to the sender, delete it from your computer, and 
>destroy any printed copy of it.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] scsi: mpt3sas: Fix NULL pointer crash due to missing check device hostdata
  2022-09-01  6:03     ` huhai
@ 2022-09-01  6:12       ` Jackie Liu
  0 siblings, 0 replies; 5+ messages in thread
From: Jackie Liu @ 2022-09-01  6:12 UTC (permalink / raw)
  To: huhai, Sathya Prakash Veerichetty
  Cc: jejb, Martin K. Petersen, Sreekanth Reddy,
	Suganath Prabu Subramani, PDL-MPT-FUSIONLINUX, linux-scsi,
	linux-kernel, huhai, stable

hu hai:

I think Sathya means this:

diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c 
b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index def37a7e5980..2a8c1fef1d34 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -5704,27 +5704,31 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16 
smid, u8 msix_index, u32 reply)
         struct MPT3SAS_DEVICE *sas_device_priv_data;
         u32 response_code = 0;

-       mpi_reply = mpt3sas_base_get_reply_virt_addr(ioc, reply);
-
         scmd = mpt3sas_scsih_scsi_lookup_get(ioc, smid);
         if (scmd == NULL)
                 return 1;

+       sas_device_priv_data = scmd->device->hostdata;
+       if (!sas_device_priv_data || !sas_device_priv_data->sas_target)
+               scmd->result = DID_NO_CONNECT << 16;
+               goto out;
+       }
+
         _scsih_set_satl_pending(scmd, false);

+       if (sas_device_priv_data->sas_target->deleted) {
+               scmd->result = DID_NO_CONNECT << 16;
+               goto out;
+       }
+
         mpi_request = mpt3sas_base_get_msg_frame(ioc, smid);

+       mpi_reply = mpt3sas_base_get_reply_virt_addr(ioc, reply);
         if (mpi_reply == NULL) {
                 scmd->result = DID_OK << 16;
                 goto out;
         }

-       sas_device_priv_data = scmd->device->hostdata;
-       if (!sas_device_priv_data || !sas_device_priv_data->sas_target ||
-            sas_device_priv_data->sas_target->deleted) {
-               scmd->result = DID_NO_CONNECT << 16;
-               goto out;
-       }
         ioc_status = le16_to_cpu(mpi_reply->IOCStatus);

         /*

-- 
Jackie Liu


在 2022/9/1 14:03, huhai 写道:
> 
> At 2022-09-01 13:08:14, "Sathya Prakash Veerichetty" <sathya.prakash@broadcom.com> wrote:
>> The patch could be improved to clear the ata_cmd_pending bit for the
>> cases !sas_device_priv_data->sas_target and
>> sas_device_priv_data->sas_target->deleted before returning
> 
>> DID_NO_CONNECT to retain the current functionality.
> 
> Hi,
> 
> Maybe my commit information is not clear enough,This patch is fixed NULL pointer crash
> duto to "struct MPT3SAS_DEVICE *priv = scmd->device->hostdata;"  got a NULL pointer, and
> when "clear_bit(0, &priv->ata_command_pending);" is called, kernel will panic.
> 
> Thanks
> 
>>
>>
>> On Wed, Aug 31, 2022 at 7:11 PM huhai <15815827059@163.com> wrote:
>>>
>>> Friendly ping.
>>>
>>>
>>> At 2022-08-25 17:26:45, "huhai" <15815827059@163.com> wrote:
>>>> From: huhai <huhai@kylinos.cn>
>>>>
>>>> If _scsih_io_done() is called with scmd->device->hostdata=NULL, it can lead
>>>> to the following panic:
>>>>
>>>>   BUG: unable to handle kernel NULL pointer dereference at 0000000000000018
>>>>   PGD 4547a4067 P4D 4547a4067 PUD 0
>>>>   Oops: 0002 [#1] SMP NOPTI
>>>>   CPU: 62 PID: 0 Comm: swapper/62 Kdump: loaded Not tainted 4.19.90-24.4.v2101.ky10.x86_64 #1
>>>>   Hardware name: Storage Server/65N32-US, BIOS SQL1041217 05/30/2022
>>>>   RIP: 0010:_scsih_set_satl_pending+0x2d/0x50 [mpt3sas]
>>>>   Code: 00 00 48 8b 87 60 01 00 00 0f b6 10 80 fa a1 74 09 31 c0 80 fa 85 74 02 f3 c3 48 8b 47 38 40 84 f6 48 8b 80 98 00 00 00 75 08 <f0> 80 60 18 fe 31 c0 c3 f0 48 0f ba 68 18 00 0f 92 c0 0f b6 c0 c3
>>>>   RSP: 0018:ffff8ec22fc03e00 EFLAGS: 00010046
>>>>   RAX: 0000000000000000 RBX: ffff8eba1b072518 RCX: 0000000000000001
>>>>   RDX: 0000000000000085 RSI: 0000000000000000 RDI: ffff8eba1b072518
>>>>   RBP: 0000000000000dbd R08: 0000000000000000 R09: 0000000000029700
>>>>   R10: ffff8ec22fc03f80 R11: 0000000000000000 R12: ffff8ebe2d3609e8
>>>>   R13: ffff8ebe2a72b600 R14: ffff8eca472707e0 R15: 0000000000000020
>>>>   FS:  0000000000000000(0000) GS:ffff8ec22fc00000(0000) knlGS:0000000000000000
>>>>   CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>>>   CR2: 0000000000000018 CR3: 000000046e5f6000 CR4: 00000000003406e0
>>>>   Call Trace:
>>>>    <IRQ>
>>>>    _scsih_io_done+0x4a/0x9f0 [mpt3sas]
>>>>    _base_interrupt+0x23f/0xe10 [mpt3sas]
>>>>    __handle_irq_event_percpu+0x40/0x190
>>>>    handle_irq_event_percpu+0x30/0x70
>>>>    handle_irq_event+0x36/0x60
>>>>    handle_edge_irq+0x7e/0x190
>>>>    handle_irq+0xa8/0x110
>>>>    do_IRQ+0x49/0xe0
>>>>
>>>> Fix it by move scmd->device->hostdata check before _scsih_set_satl_pending
>>>> called.
>>>>
>>>> Other changes:
>>>> - It looks clear to move get mpi_reply to near its check.
>>>>
>>>> Fixes: ffb584565894 ("scsi: mpt3sas: fix hang on ata passthrough commands")
>>>> Cc: <stable@vger.kernel.org> # v4.9+
>>>> Co-developed-by: Jackie Liu <liuyun01@kylinos.cn>
>>>> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
>>>> Signed-off-by: huhai <huhai@kylinos.cn>
>>>> ---
>>>> drivers/scsi/mpt3sas/mpt3sas_scsih.c | 15 +++++++--------
>>>> 1 file changed, 7 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
>>>> index def37a7e5980..85f5749a0421 100644
>>>> --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
>>>> +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
>>>> @@ -5704,27 +5704,26 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16 smid, u8 msix_index, u32 reply)
>>>>        struct MPT3SAS_DEVICE *sas_device_priv_data;
>>>>        u32 response_code = 0;
>>>>
>>>> -      mpi_reply = mpt3sas_base_get_reply_virt_addr(ioc, reply);
>>>> -
>>>>        scmd = mpt3sas_scsih_scsi_lookup_get(ioc, smid);
>>>>        if (scmd == NULL)
>>>>                return 1;
>>>>
>>>> +      sas_device_priv_data = scmd->device->hostdata;
>>>> +      if (!sas_device_priv_data || !sas_device_priv_data->sas_target ||
>>>> +           sas_device_priv_data->sas_target->deleted) {
>>>> +              scmd->result = DID_NO_CONNECT << 16;
>>>> +              goto out;
>>>> +      }
>>>>        _scsih_set_satl_pending(scmd, false);
>>>>
>>>>        mpi_request = mpt3sas_base_get_msg_frame(ioc, smid);
>>>>
>>>> +      mpi_reply = mpt3sas_base_get_reply_virt_addr(ioc, reply);
>>>>        if (mpi_reply == NULL) {
>>>>                scmd->result = DID_OK << 16;
>>>>                goto out;
>>>>        }
>>>>
>>>> -      sas_device_priv_data = scmd->device->hostdata;
>>>> -      if (!sas_device_priv_data || !sas_device_priv_data->sas_target ||
>>>> -           sas_device_priv_data->sas_target->deleted) {
>>>> -              scmd->result = DID_NO_CONNECT << 16;
>>>> -              goto out;
>>>> -      }
>>>>        ioc_status = le16_to_cpu(mpi_reply->IOCStatus);
>>>>
>>>>        /*
>>>> --
>>>> 2.27.0
>>>>
>>>>
>>>> No virus found
>>>>                Checked by Hillstone Network AntiVirus
>>
>> -- 
>> This electronic communication and the information and any files transmitted
>> with it, or attached to it, are confidential and are intended solely for
>> the use of the individual or entity to whom it is addressed and may contain
>> information that is confidential, legally privileged, protected by privacy
>> laws, or otherwise restricted from disclosure to anyone else. If you are
>> not the intended recipient or the person responsible for delivering the
>> e-mail to the intended recipient, you are hereby notified that any use,
>> copying, distributing, dissemination, forwarding, printing, or copying of
>> this e-mail is strictly prohibited. If you received this e-mail in error,
>> please return the e-mail to the sender, delete it from your computer, and
>> destroy any printed copy of it.

^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-09-01  6:12 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-25  9:26 [PATCH] scsi: mpt3sas: Fix NULL pointer crash due to missing check device hostdata huhai
2022-09-01  1:11 ` huhai
2022-09-01  5:08   ` [PATCH] " Sathya Prakash Veerichetty
2022-09-01  6:03     ` huhai
2022-09-01  6:12       ` Jackie Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).