| Message ID | 20220825092645.326953-1-15815827059@163.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | scsi: mpt3sas: Fix NULL pointer crash due to missing check device hostdata | expand |
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
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
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.
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);
/*
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); /*