From 9b4f5028e493cb353a5c8f5c45073eeea0303abd Mon Sep 17 00:00:00 2001 From: Tomas Henzl Date: Thu, 2 Feb 2023 17:24:48 +0100 Subject: scsi: ses: Fix slab-out-of-bounds in ses_enclosure_data_process() A fix for: BUG: KASAN: slab-out-of-bounds in ses_enclosure_data_process+0x949/0xe30 [ses] Read of size 1 at addr ffff88a1b043a451 by task systemd-udevd/3271 Checking after (and before in next loop) addl_desc_ptr[1] is sufficient, we expect the size to be sanitized before first access to addl_desc_ptr[1]. Make sure we don't walk beyond end of page. Link: https://lore.kernel.org/r/20230202162451.15346-2-thenzl@redhat.com Cc: stable@vger.kernel.org Signed-off-by: Tomas Henzl Signed-off-by: Martin K. Petersen --- drivers/scsi/ses.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c index 869ca9c7f23f..034d605c3076 100644 --- a/drivers/scsi/ses.c +++ b/drivers/scsi/ses.c @@ -609,9 +609,11 @@ static void ses_enclosure_data_process(struct enclosure_device *edev, /* these elements are optional */ type_ptr[0] == ENCLOSURE_COMPONENT_SCSI_TARGET_PORT || type_ptr[0] == ENCLOSURE_COMPONENT_SCSI_INITIATOR_PORT || - type_ptr[0] == ENCLOSURE_COMPONENT_CONTROLLER_ELECTRONICS)) + type_ptr[0] == ENCLOSURE_COMPONENT_CONTROLLER_ELECTRONICS)) { addl_desc_ptr += addl_desc_ptr[1] + 2; - + if (addl_desc_ptr + 1 >= ses_dev->page10 + ses_dev->page10_len) + addl_desc_ptr = NULL; + } } } kfree(buf); -- cgit v1.2.3 From db95d4df71cb55506425b6e4a5f8d68e3a765b63 Mon Sep 17 00:00:00 2001 From: Tomas Henzl Date: Thu, 2 Feb 2023 17:24:49 +0100 Subject: scsi: ses: Fix possible addl_desc_ptr out-of-bounds accesses Sanitize possible addl_desc_ptr out-of-bounds accesses in ses_enclosure_data_process(). Link: https://lore.kernel.org/r/20230202162451.15346-3-thenzl@redhat.com Cc: stable@vger.kernel.org Signed-off-by: Tomas Henzl Signed-off-by: Martin K. Petersen --- drivers/scsi/ses.c | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c index 034d605c3076..458ca7abbbef 100644 --- a/drivers/scsi/ses.c +++ b/drivers/scsi/ses.c @@ -439,8 +439,8 @@ int ses_match_host(struct enclosure_device *edev, void *data) } #endif /* 0 */ -static void ses_process_descriptor(struct enclosure_component *ecomp, - unsigned char *desc) +static int ses_process_descriptor(struct enclosure_component *ecomp, + unsigned char *desc, int max_desc_len) { int eip = desc[0] & 0x10; int invalid = desc[0] & 0x80; @@ -451,22 +451,32 @@ static void ses_process_descriptor(struct enclosure_component *ecomp, unsigned char *d; if (invalid) - return; + return 0; switch (proto) { case SCSI_PROTOCOL_FCP: if (eip) { + if (max_desc_len <= 7) + return 1; d = desc + 4; slot = d[3]; } break; case SCSI_PROTOCOL_SAS: + if (eip) { + if (max_desc_len <= 27) + return 1; d = desc + 4; slot = d[3]; d = desc + 8; - } else + } else { + if (max_desc_len <= 23) + return 1; d = desc + 4; + } + + /* only take the phy0 addr */ addr = (u64)d[12] << 56 | (u64)d[13] << 48 | @@ -483,6 +493,8 @@ static void ses_process_descriptor(struct enclosure_component *ecomp, } ecomp->slot = slot; scomp->addr = addr; + + return 0; } struct efd { @@ -555,7 +567,7 @@ static void ses_enclosure_data_process(struct enclosure_device *edev, /* skip past overall descriptor */ desc_ptr += len + 4; } - if (ses_dev->page10) + if (ses_dev->page10 && ses_dev->page10_len > 9) addl_desc_ptr = ses_dev->page10 + 8; type_ptr = ses_dev->page1_types; components = 0; @@ -563,6 +575,7 @@ static void ses_enclosure_data_process(struct enclosure_device *edev, for (j = 0; j < type_ptr[1]; j++) { char *name = NULL; struct enclosure_component *ecomp; + int max_desc_len; if (desc_ptr) { if (desc_ptr >= buf + page7_len) { @@ -589,10 +602,14 @@ static void ses_enclosure_data_process(struct enclosure_device *edev, ecomp = &edev->component[components++]; if (!IS_ERR(ecomp)) { - if (addl_desc_ptr) - ses_process_descriptor( - ecomp, - addl_desc_ptr); + if (addl_desc_ptr) { + max_desc_len = ses_dev->page10_len - + (addl_desc_ptr - ses_dev->page10); + if (ses_process_descriptor(ecomp, + addl_desc_ptr, + max_desc_len)) + addl_desc_ptr = NULL; + } if (create) enclosure_component_register( ecomp); -- cgit v1.2.3 From 801ab13d50cf3d26170ee073ea8bb4eececb76ab Mon Sep 17 00:00:00 2001 From: Tomas Henzl Date: Thu, 2 Feb 2023 17:24:50 +0100 Subject: scsi: ses: Fix possible desc_ptr out-of-bounds accesses Sanitize possible desc_ptr out-of-bounds accesses in ses_enclosure_data_process(). Link: https://lore.kernel.org/r/20230202162451.15346-4-thenzl@redhat.com Cc: stable@vger.kernel.org Signed-off-by: Tomas Henzl Signed-off-by: Martin K. Petersen --- drivers/scsi/ses.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c index 458ca7abbbef..f8031d0782f7 100644 --- a/drivers/scsi/ses.c +++ b/drivers/scsi/ses.c @@ -578,15 +578,19 @@ static void ses_enclosure_data_process(struct enclosure_device *edev, int max_desc_len; if (desc_ptr) { - if (desc_ptr >= buf + page7_len) { + if (desc_ptr + 3 >= buf + page7_len) { desc_ptr = NULL; } else { len = (desc_ptr[2] << 8) + desc_ptr[3]; desc_ptr += 4; - /* Add trailing zero - pushes into - * reserved space */ - desc_ptr[len] = '\0'; - name = desc_ptr; + if (desc_ptr + len > buf + page7_len) + desc_ptr = NULL; + else { + /* Add trailing zero - pushes into + * reserved space */ + desc_ptr[len] = '\0'; + name = desc_ptr; + } } } if (type_ptr[0] == ENCLOSURE_COMPONENT_DEVICE || -- cgit v1.2.3 From 578797f0c8cbc2e3ec5fc0dab87087b4c7073686 Mon Sep 17 00:00:00 2001 From: Tomas Henzl Date: Thu, 2 Feb 2023 17:24:51 +0100 Subject: scsi: ses: Fix slab-out-of-bounds in ses_intf_remove() A fix for: BUG: KASAN: slab-out-of-bounds in ses_intf_remove+0x23f/0x270 [ses] Read of size 8 at addr ffff88a10d32e5d8 by task rmmod/12013 When edev->components is zero, accessing edev->component[0] members is wrong. Link: https://lore.kernel.org/r/20230202162451.15346-5-thenzl@redhat.com Cc: stable@vger.kernel.org Signed-off-by: Tomas Henzl Signed-off-by: Martin K. Petersen --- drivers/scsi/ses.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c index f8031d0782f7..9d4fb09acc1e 100644 --- a/drivers/scsi/ses.c +++ b/drivers/scsi/ses.c @@ -856,7 +856,8 @@ static void ses_intf_remove_enclosure(struct scsi_device *sdev) kfree(ses_dev->page2); kfree(ses_dev); - kfree(edev->component[0].scratch); + if (edev->components) + kfree(edev->component[0].scratch); put_device(&edev->edev); enclosure_unregister(edev); -- cgit v1.2.3 From 3fe97ff3d94934649abb0652028dd7296170c8d0 Mon Sep 17 00:00:00 2001 From: James Bottomley Date: Sat, 28 Nov 2020 15:27:21 -0800 Subject: scsi: ses: Don't attach if enclosure has no components An enclosure with no components can't usefully be operated by the driver (since effectively it has nothing to manage), so report the problem and don't attach. Not attaching also fixes an oops which could occur if the driver tries to manage a zero component enclosure. [mkp: Switched to KERN_WARNING since this scenario is common] Link: https://lore.kernel.org/r/c5deac044ac409e32d9ad9968ce0dcbc996bfc7a.camel@linux.ibm.com Cc: stable@vger.kernel.org Reported-by: Ding Hui Signed-off-by: James Bottomley Signed-off-by: Martin K. Petersen --- drivers/scsi/ses.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c index 9d4fb09acc1e..b11a9162e73a 100644 --- a/drivers/scsi/ses.c +++ b/drivers/scsi/ses.c @@ -733,6 +733,12 @@ static int ses_intf_add(struct device *cdev, type_ptr[0] == ENCLOSURE_COMPONENT_ARRAY_DEVICE) components += type_ptr[1]; } + + if (components == 0) { + sdev_printk(KERN_WARNING, sdev, "enclosure has no enumerated components\n"); + goto err_free; + } + ses_dev->page1 = buf; ses_dev->page1_len = len; buf = NULL; -- cgit v1.2.3 From 64fd2ba977b1b6b073bcf4c71906f820bc531421 Mon Sep 17 00:00:00 2001 From: Muneendra Date: Wed, 8 Feb 2023 19:43:26 -0800 Subject: scsi: scsi_transport_fc: Add an additional flag to fc_host_fpin_rcv() The LLDD and the stack currently process FPINs received from the fabric, but the stack is not aware of any action taken by the driver to alleviate congestion. The current interface between the driver and the SCSI stack is limited to passing the notification mainly for statistics and heuristics. The reaction to an FPIN could be handled either by the driver or by the stack (marginal path and failover). Amend the interface to indicate if action on an FPIN has already been reacted to by the LLDDs or not. Add an additional flag to fc_host_fpin_rcv() to indicate if the FPIN has been acknowledged/reacted to by the driver. Also added a new event code FCH_EVT_LINK_FPIN_ACK to notify to the user that the event has been acknowledged/reacted by the LLDD driver Link: https://lore.kernel.org/r/20230209034326.882514-1-muneendra.kumar@broadcom.com Co-developed-by: Anil Gurumurthy Signed-off-by: Anil Gurumurthy Co-developed-by: Nilesh Javali Signed-off-by: Nilesh Javali Signed-off-by: Muneendra Reviewed-by: James Smart Reviewed-by: Himanshu Madhani Reviewed-by: Ewan D. Milne Signed-off-by: Martin K. Petersen --- drivers/scsi/lpfc/lpfc_els.c | 2 +- drivers/scsi/qla2xxx/qla_isr.c | 2 +- drivers/scsi/scsi_transport_fc.c | 10 +++++++--- include/scsi/scsi_transport_fc.h | 4 +++- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c index 569639dc8b2c..aee5d0d1187d 100644 --- a/drivers/scsi/lpfc/lpfc_els.c +++ b/drivers/scsi/lpfc/lpfc_els.c @@ -10287,7 +10287,7 @@ lpfc_els_rcv_fpin(struct lpfc_vport *vport, void *p, u32 fpin_length) /* Send every descriptor individually to the upper layer */ if (deliver) fc_host_fpin_rcv(lpfc_shost_from_vport(vport), - fpin_length, (char *)fpin); + fpin_length, (char *)fpin, 0); desc_cnt++; } } diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c index 46e8b38603f0..030625ebb4e6 100644 --- a/drivers/scsi/qla2xxx/qla_isr.c +++ b/drivers/scsi/qla2xxx/qla_isr.c @@ -45,7 +45,7 @@ qla27xx_process_purex_fpin(struct scsi_qla_host *vha, struct purex_item *item) ql_dump_buffer(ql_dbg_init + ql_dbg_verbose, vha, 0x508f, pkt, pkt_size); - fc_host_fpin_rcv(vha->host, pkt_size, (char *)pkt); + fc_host_fpin_rcv(vha->host, pkt_size, (char *)pkt, 0); } const char *const port_state_str[] = { diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c index 0965f8a7134f..f12e9467ebb4 100644 --- a/drivers/scsi/scsi_transport_fc.c +++ b/drivers/scsi/scsi_transport_fc.c @@ -137,6 +137,7 @@ static const struct { { FCH_EVT_PORT_FABRIC, "port_fabric" }, { FCH_EVT_LINK_UNKNOWN, "link_unknown" }, { FCH_EVT_LINK_FPIN, "link_FPIN" }, + { FCH_EVT_LINK_FPIN_ACK, "link_FPIN_ACK" }, { FCH_EVT_VENDOR_UNIQUE, "vendor_unique" }, }; fc_enum_name_search(host_event_code, fc_host_event_code, @@ -894,17 +895,20 @@ fc_fpin_congn_stats_update(struct Scsi_Host *shost, * @shost: host the FPIN was received on * @fpin_len: length of FPIN payload, in bytes * @fpin_buf: pointer to FPIN payload - * + * @event_acknowledge: 1, if LLDD handles this event. * Notes: * This routine assumes no locks are held on entry. */ void -fc_host_fpin_rcv(struct Scsi_Host *shost, u32 fpin_len, char *fpin_buf) +fc_host_fpin_rcv(struct Scsi_Host *shost, u32 fpin_len, char *fpin_buf, + u8 event_acknowledge) { struct fc_els_fpin *fpin = (struct fc_els_fpin *)fpin_buf; struct fc_tlv_desc *tlv; u32 desc_cnt = 0, bytes_remain; u32 dtag; + enum fc_host_event_code event_code = + event_acknowledge ? FCH_EVT_LINK_FPIN_ACK : FCH_EVT_LINK_FPIN; /* Update Statistics */ tlv = (struct fc_tlv_desc *)&fpin->fpin_desc[0]; @@ -934,7 +938,7 @@ fc_host_fpin_rcv(struct Scsi_Host *shost, u32 fpin_len, char *fpin_buf) } fc_host_post_fc_event(shost, fc_get_event_number(), - FCH_EVT_LINK_FPIN, fpin_len, fpin_buf, 0); + event_code, fpin_len, fpin_buf, 0); } EXPORT_SYMBOL(fc_host_fpin_rcv); diff --git a/include/scsi/scsi_transport_fc.h b/include/scsi/scsi_transport_fc.h index 3dcda19d3520..483513c57597 100644 --- a/include/scsi/scsi_transport_fc.h +++ b/include/scsi/scsi_transport_fc.h @@ -496,6 +496,7 @@ enum fc_host_event_code { FCH_EVT_PORT_FABRIC = 0x204, FCH_EVT_LINK_UNKNOWN = 0x500, FCH_EVT_LINK_FPIN = 0x501, + FCH_EVT_LINK_FPIN_ACK = 0x502, FCH_EVT_VENDOR_UNIQUE = 0xffff, }; @@ -856,7 +857,8 @@ void fc_host_post_fc_event(struct Scsi_Host *shost, u32 event_number, * Note: when calling fc_host_post_fc_event(), vendor_id may be * specified as 0. */ -void fc_host_fpin_rcv(struct Scsi_Host *shost, u32 fpin_len, char *fpin_buf); +void fc_host_fpin_rcv(struct Scsi_Host *shost, u32 fpin_len, char *fpin_buf, + u8 event_acknowledge); struct fc_vport *fc_vport_create(struct Scsi_Host *shost, int channel, struct fc_vport_identifiers *); int fc_vport_terminate(struct fc_vport *vport); -- cgit v1.2.3 From 473025a6b6f8a4a8de3120c78588d32cf4ba7324 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Thu, 9 Feb 2023 10:49:03 -0800 Subject: scsi: ufs: Make the TC G210 driver dependent on CONFIG_OF MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The TC G210 driver only supports devices declared in the device tree. Hence make this driver dependent on CONFIG_OF. This patch fixes the following compiler error: drivers/ufs/host/tc-dwc-g210-pltfrm.c:36:34: error: ‘tc_dwc_g210_pltfm_match’ defined but not used [-Werror=unused-const-variable=] 36 | static const struct of_device_id tc_dwc_g210_pltfm_match[] = { | Link: https://lore.kernel.org/r/20230209184914.2762172-1-bvanassche@acm.org Cc: Joao Pinto Signed-off-by: Bart Van Assche Reviewed-by: Bean Huo Signed-off-by: Martin K. Petersen --- drivers/ufs/host/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/ufs/host/Kconfig b/drivers/ufs/host/Kconfig index 663881437921..8793e3433580 100644 --- a/drivers/ufs/host/Kconfig +++ b/drivers/ufs/host/Kconfig @@ -48,7 +48,7 @@ config SCSI_UFS_CDNS_PLATFORM config SCSI_UFS_DWC_TC_PLATFORM tristate "DesignWare platform support using a G210 Test Chip" - depends on SCSI_UFSHCD_PLATFORM + depends on OF && SCSI_UFSHCD_PLATFORM help Synopsys Test Chip is a PHY for prototyping purposes. -- cgit v1.2.3 From 7dafc3e007918384c8693ff8d70381b5c1e9c247 Mon Sep 17 00:00:00 2001 From: Adrien Thierry Date: Fri, 17 Feb 2023 14:44:22 -0500 Subject: scsi: ufs: core: Initialize devfreq synchronously During UFS initialization, devfreq initialization is asynchronous: ufshcd_async_scan() calls ufshcd_add_lus(), which in turn initializes devfreq for UFS. The simple ondemand governor is then loaded. If it is built as a module, request_module() is called and throws a warning: WARNING: CPU: 7 PID: 167 at kernel/kmod.c:136 __request_module+0x1e0/0x460 Modules linked in: crct10dif_ce llcc_qcom phy_qcom_qmp_usb ufs_qcom phy_qcom_snps_femto_v2 ufshcd_pltfrm phy_qcom_qmp_combo ufshcd_core phy_qcom_qmp_ufs qcom_wdt socinfo fuse ipv6 CPU: 7 PID: 167 Comm: kworker/u16:3 Not tainted 6.2.0-rc6-00009-g58706f7fb045 #1 Hardware name: Qualcomm SA8540P Ride (DT) Workqueue: events_unbound async_run_entry_fn pstate: 00400005 (nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) pc : __request_module+0x1e0/0x460 lr : __request_module+0x1d8/0x460 sp : ffff800009323b90 x29: ffff800009323b90 x28: 0000000000000000 x27: 0000000000000000 x26: ffff800009323d50 x25: ffff7b9045f57810 x24: ffff7b9045f57830 x23: ffffdc5a83e426e8 x22: ffffdc5ae80a9818 x21: 0000000000000001 x20: ffffdc5ae7502f98 x19: ffff7b9045f57800 x18: ffffffffffffffff x17: 312f716572667665 x16: 642f7366752e3030 x15: 0000000000000000 x14: 000000000000021c x13: 0000000000005400 x12: ffff7b9042ed7614 x11: ffff7b9042ed7600 x10: 00000000636c0890 x9 : 0000000000000038 x8 : ffff7b9045f2c880 x7 : ffff7b9045f57c68 x6 : 0000000000000080 x5 : 0000000000000000 x4 : 8000000000000000 x3 : 0000000000000000 x2 : 0000000000000000 x1 : ffffdc5ae5d382f0 x0 : 0000000000000001 Call trace: __request_module+0x1e0/0x460 try_then_request_governor+0x7c/0x100 devfreq_add_device+0x4b0/0x5fc ufshcd_async_scan+0x1d4/0x310 [ufshcd_core] async_run_entry_fn+0x34/0xe0 process_one_work+0x1d0/0x320 worker_thread+0x14c/0x444 kthread+0x10c/0x110 ret_from_fork+0x10/0x20 This occurs because synchronous module loading from async is not allowed. According to __request_module(): /* * We don't allow synchronous module loading from async. Module * init may invoke async_synchronize_full() which will end up * waiting for this task which already is waiting for the module * loading to complete, leading to a deadlock. */ Such a deadlock was experienced on the Qualcomm QDrive3/sa8540p-ride. With DEVFREQ_GOV_SIMPLE_ONDEMAND=m, the boot hangs after the warning. Fix both the warning and the deadlock by moving devfreq initialization out of the async routine. Tested on the sa8540p-ride by using fio to put the UFS under load, and printing the trace generated by /sys/kernel/tracing/events/ufs/ufshcd_clk_scaling events. The trace looks similar with and without the change. Link: https://lore.kernel.org/r/20230217194423.42553-1-athierry@redhat.com Signed-off-by: Adrien Thierry Reviewed-by: Bart Van Assche Signed-off-by: Martin K. Petersen --- drivers/ufs/core/ufshcd.c | 47 +++++++++++++++++++++++++++++++---------------- include/ufs/ufshcd.h | 1 + 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index 3b3cf78d3b10..0baeec4ca304 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -1411,6 +1411,13 @@ static int ufshcd_devfreq_target(struct device *dev, struct ufs_clk_info *clki; unsigned long irq_flags; + /* + * Skip devfreq if UFS initialization is not finished. + * Otherwise ufs could be in a inconsistent state. + */ + if (!smp_load_acquire(&hba->logical_unit_scan_finished)) + return 0; + if (!ufshcd_is_clkscaling_supported(hba)) return -EINVAL; @@ -8384,22 +8391,6 @@ static int ufshcd_add_lus(struct ufs_hba *hba) if (ret) goto out; - /* Initialize devfreq after UFS device is detected */ - if (ufshcd_is_clkscaling_supported(hba)) { - memcpy(&hba->clk_scaling.saved_pwr_info.info, - &hba->pwr_info, - sizeof(struct ufs_pa_layer_attr)); - hba->clk_scaling.saved_pwr_info.is_valid = true; - hba->clk_scaling.is_allowed = true; - - ret = ufshcd_devfreq_init(hba); - if (ret) - goto out; - - hba->clk_scaling.is_enabled = true; - ufshcd_init_clk_scaling_sysfs(hba); - } - ufs_bsg_probe(hba); ufshpb_init(hba); scsi_scan_host(hba->host); @@ -8669,6 +8660,12 @@ out: if (ret) { pm_runtime_put_sync(hba->dev); ufshcd_hba_exit(hba); + } else { + /* + * Make sure that when reader code sees UFS initialization has finished, + * all initialization steps have really been executed. + */ + smp_store_release(&hba->logical_unit_scan_finished, true); } } @@ -10309,12 +10306,30 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq) */ ufshcd_set_ufs_dev_active(hba); + /* Initialize devfreq */ + if (ufshcd_is_clkscaling_supported(hba)) { + memcpy(&hba->clk_scaling.saved_pwr_info.info, + &hba->pwr_info, + sizeof(struct ufs_pa_layer_attr)); + hba->clk_scaling.saved_pwr_info.is_valid = true; + hba->clk_scaling.is_allowed = true; + + err = ufshcd_devfreq_init(hba); + if (err) + goto rpm_put_sync; + + hba->clk_scaling.is_enabled = true; + ufshcd_init_clk_scaling_sysfs(hba); + } + async_schedule(ufshcd_async_scan, hba); ufs_sysfs_add_nodes(hba->dev); device_enable_async_suspend(dev); return 0; +rpm_put_sync: + pm_runtime_put_sync(dev); free_tmf_queue: blk_mq_destroy_queue(hba->tmf_queue); blk_put_queue(hba->tmf_queue); diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h index ed9e3d5addb3..05e416414e41 100644 --- a/include/ufs/ufshcd.h +++ b/include/ufs/ufshcd.h @@ -978,6 +978,7 @@ struct ufs_hba { struct completion *uic_async_done; enum ufshcd_state ufshcd_state; + bool logical_unit_scan_finished; u32 eh_flags; u32 intr_mask; u16 ee_ctrl_mask; -- cgit v1.2.3 From 19873b03f1daeffd9d5e089b855dd926975ab5b7 Mon Sep 17 00:00:00 2001 From: Yangtao Li Date: Mon, 20 Feb 2023 22:24:31 +0800 Subject: scsi: ufs: ufs-mediatek: Guard power management functions with CONFIG_PM Fix the following compilation error when CONFIG_PM is set to 'n': drivers/ufs/host/ufs-mediatek.c: In function `ufs_mtk_runtime_suspend`: drivers/ufs/host/ufs-mediatek.c:1623:8: error: implicit declaration of function `ufshcd_runtime_suspend`; did you mean `ufs_mtk_runtime_suspend`? [-Werror=implicit-function-declaration] 1623 | ret = ufshcd_runtime_suspend(dev); | ^~~~~~~~~~~~~~~~~~~~~~ | ufs_mtk_runtime_suspend drivers/ufs/host/ufs-mediatek.c: In function `ufs_mtk_runtime_resume`: drivers/ufs/host/ufs-mediatek.c:1638:9: error: implicit declaration of function `ufshcd_runtime_resume`; did you mean `ufs_mtk_runtime_resume`? [-Werror=implicit-function-declaration] 1638 | return ufshcd_runtime_resume(dev); | ^~~~~~~~~~~~~~~~~~~~~ | ufs_mtk_runtime_resume At top level: drivers/ufs/host/ufs-mediatek.c:1632:12: error: `ufs_mtk_runtime_resume` defined but not used [-Werror=unused-function] 1632 | static int ufs_mtk_runtime_resume(struct device *dev) | ^~~~~~~~~~~~~~~~~~~~~~ drivers/ufs/host/ufs-mediatek.c:1618:12: error: `ufs_mtk_runtime_suspend` defined but not used [-Werror=unused-function] 1618 | static int ufs_mtk_runtime_suspend(struct device *dev) Link: https://lore.kernel.org/r/20230220142431.54589-1-frank.li@vivo.com Reported-by: k2ci Reported-by: Shida Zhang Signed-off-by: Yangtao Li Reviewed-by: Stanley Chu Signed-off-by: Martin K. Petersen --- drivers/ufs/host/ufs-mediatek.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c index 21d9b047539f..73e217260390 100644 --- a/drivers/ufs/host/ufs-mediatek.c +++ b/drivers/ufs/host/ufs-mediatek.c @@ -1613,6 +1613,7 @@ static int ufs_mtk_system_resume(struct device *dev) } #endif +#ifdef CONFIG_PM static int ufs_mtk_runtime_suspend(struct device *dev) { struct ufs_hba *hba = dev_get_drvdata(dev); @@ -1635,6 +1636,7 @@ static int ufs_mtk_runtime_resume(struct device *dev) return ufshcd_runtime_resume(dev); } +#endif static const struct dev_pm_ops ufs_mtk_pm_ops = { SET_SYSTEM_SLEEP_PM_OPS(ufs_mtk_system_suspend, -- cgit v1.2.3 From 2076f57f2ca0a2034afac7832257b2bc509c1a87 Mon Sep 17 00:00:00 2001 From: Asutosh Das Date: Thu, 16 Feb 2023 09:13:46 -0800 Subject: scsi: ufs: mcq: Fix incorrectly set queue depth ufshcd_config_mcq() may change the can_queue value. The current code invokes scsi_add_host() before ufshcd_config_mcq() so the tags are limited to the original can_queue value. Fix this by invoking scsi_add_host() after ufshcd_config_mcq(). Link: https://lore.kernel.org/r/8840cea4a57b46dabce18acc39afc50ab826330f.1676567593.git.quic_asutoshd@quicinc.com Fixes: 2468da61ea09 ("scsi: ufs: core: mcq: Configure operation and runtime interface") Signed-off-by: Asutosh Das Reviewed-by: Bart Van Assche Reviewed-by: Manivannan Sadhasivam Signed-off-by: Martin K. Petersen --- drivers/ufs/core/ufshcd.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index 0baeec4ca304..47c7739b9a86 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -8521,7 +8521,9 @@ static int ufshcd_device_init(struct ufs_hba *hba, bool init_dev_params) return ret; if (is_mcq_supported(hba) && !hba->scsi_host_added) { ret = ufshcd_alloc_mcq(hba); - if (ret) { + if (!ret) { + ufshcd_config_mcq(hba); + } else { /* Continue with SDB mode */ use_mcq_mode = false; dev_err(hba->dev, "MCQ mode is disabled, err=%d\n", @@ -8533,10 +8535,10 @@ static int ufshcd_device_init(struct ufs_hba *hba, bool init_dev_params) return ret; } hba->scsi_host_added = true; - } - /* MCQ may be disabled if ufshcd_alloc_mcq() fails */ - if (is_mcq_supported(hba) && use_mcq_mode) + } else if (is_mcq_supported(hba)) { + /* UFSHCD_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH is set */ ufshcd_config_mcq(hba); + } } ufshcd_tune_unipro_params(hba); -- cgit v1.2.3 From 2b3795167e233dc9974f9f8239afb920f5cd4a32 Mon Sep 17 00:00:00 2001 From: Fengnan Chang Date: Tue, 21 Feb 2023 19:53:40 +0800 Subject: scsi: sd: Remove unused sd_cdb_cache Since commit ce70fd9a551a ("scsi: core: Remove the cmd field from struct scsi_request") sd_cdb_cache is unused. Remove it. Link: https://lore.kernel.org/r/20230221115340.21201-1-changfengnan@bytedance.com Signed-off-by: Fengnan Chang Reviewed-by: John Garry Reviewed-by: Christoph Hellwig Signed-off-by: Martin K. Petersen --- drivers/scsi/sd.c | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index 2aa3b0393b96..a9f7e019e073 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -121,7 +121,6 @@ static void scsi_disk_release(struct device *cdev); static DEFINE_IDA(sd_index_ida); -static struct kmem_cache *sd_cdb_cache; static mempool_t *sd_page_pool; static struct lock_class_key sd_bio_compl_lkclass; @@ -3853,19 +3852,11 @@ static int __init init_sd(void) if (err) goto err_out; - sd_cdb_cache = kmem_cache_create("sd_ext_cdb", SD_EXT_CDB_SIZE, - 0, 0, NULL); - if (!sd_cdb_cache) { - printk(KERN_ERR "sd: can't init extended cdb cache\n"); - err = -ENOMEM; - goto err_out_class; - } - sd_page_pool = mempool_create_page_pool(SD_MEMPOOL_SIZE, 0); if (!sd_page_pool) { printk(KERN_ERR "sd: can't init discard page pool\n"); err = -ENOMEM; - goto err_out_cache; + goto err_out_class; } err = scsi_register_driver(&sd_template.gendrv); @@ -3876,10 +3867,6 @@ static int __init init_sd(void) err_out_driver: mempool_destroy(sd_page_pool); - -err_out_cache: - kmem_cache_destroy(sd_cdb_cache); - err_out_class: class_unregister(&sd_disk_class); err_out: @@ -3901,7 +3888,6 @@ static void __exit exit_sd(void) scsi_unregister_driver(&sd_template.gendrv); mempool_destroy(sd_page_pool); - kmem_cache_destroy(sd_cdb_cache); class_unregister(&sd_disk_class); -- cgit v1.2.3 From 26a02d972bad946ecbaa12131736c8b6d28893f8 Mon Sep 17 00:00:00 2001 From: Xingui Yang Date: Tue, 21 Feb 2023 08:10:26 +0000 Subject: scsi: sd: Update DIX config every time sd_revalidate_disk() is called If a controller has DIX is enabled and an attached disk is formatted using a protection type supported by the controller, a block integrity profile is registered to enable protected transfers. If the disk is subsequently reformatted to disable PI, and the controller does not support DIX Type 0, this can lead to failures such as this: [142829.032340] hisi_sas_v3_hw 0000:b4:04.0: erroneous completion iptt=2375 task=00000000bea0970c dev id=5 direct-attached phy4 addr=51c20dbaf642a000 CQ hdr: 0x1023 0x50947 0x0 0x20000 Error info: 0x0 0x0 0x4 0x0 [142829.073883] sas: Enter sas_scsi_recover_host busy: 1 failed: 1 [142829.079783] sas: sas_scsi_find_task: aborting task 0x00000000bea0970c [142829.102342] sas: Internal abort: task to dev 51c20dbaf642a000 response: 0x0 status 0x5 [142829.110319] sas: sas_eh_handle_sas_errors: task 0x00000000bea0970c is done [142829.117275] sd 7:0:5:0: [sdc] tag#2375 UNKNOWN(0x2003) Result: hostbyte=0x05 driverbyte=DRIVER_OK cmd_age=0s [142829.127171] sd 7:0:5:0: [sdc] tag#2375 CDB: opcode=0x2a 2a 00 00 00 00 00 00 00 08 00 [142829.135059] I/O error, dev sdc, sector 0 op 0x1:(WRITE) flags 0x18800 phys_seg 1 prio class 2 This is because the block layer integrity profile is currently only set up the first time a disk is discovered. To address this, remove the first_scan check when configuring protection information during revalidate. Also unregister the block integrity profile if DIX is not supported with a given protection type. [mkp: commit description + printk dedup] Link: https://lore.kernel.org/r/20230221081026.24736-1-yangxingui@huawei.com Signed-off-by: Xingui Yang Signed-off-by: Martin K. Petersen --- drivers/scsi/sd.c | 13 +++++-------- drivers/scsi/sd_dif.c | 10 ++++++---- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index a9f7e019e073..2f9b7c8f6bd0 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -2253,23 +2253,20 @@ static void sd_config_protection(struct scsi_disk *sdkp) { struct scsi_device *sdp = sdkp->device; - if (!sdkp->first_scan) - return; - sd_dif_config_host(sdkp); if (!sdkp->protection_type) return; if (!scsi_host_dif_capable(sdp->host, sdkp->protection_type)) { - sd_printk(KERN_NOTICE, sdkp, - "Disabling DIF Type %u protection\n", - sdkp->protection_type); + sd_first_printk(KERN_NOTICE, sdkp, + "Disabling DIF Type %u protection\n", + sdkp->protection_type); sdkp->protection_type = 0; } - sd_printk(KERN_NOTICE, sdkp, "Enabling DIF Type %u protection\n", - sdkp->protection_type); + sd_first_printk(KERN_NOTICE, sdkp, "Enabling DIF Type %u protection\n", + sdkp->protection_type); } static void read_capacity_error(struct scsi_disk *sdkp, struct scsi_device *sdp, diff --git a/drivers/scsi/sd_dif.c b/drivers/scsi/sd_dif.c index 968993ee6d5d..1df847b5f747 100644 --- a/drivers/scsi/sd_dif.c +++ b/drivers/scsi/sd_dif.c @@ -39,8 +39,10 @@ void sd_dif_config_host(struct scsi_disk *sdkp) dif = 0; dix = 1; } - if (!dix) + if (!dix) { + blk_integrity_unregister(disk); return; + } memset(&bi, 0, sizeof(bi)); @@ -72,9 +74,9 @@ void sd_dif_config_host(struct scsi_disk *sdkp) bi.tag_size = sizeof(u16); } - sd_printk(KERN_NOTICE, sdkp, - "Enabling DIX %s, application tag size %u bytes\n", - bi.profile->name, bi.tag_size); + sd_first_printk(KERN_NOTICE, sdkp, + "Enabling DIX %s, application tag size %u bytes\n", + bi.profile->name, bi.tag_size); out: blk_integrity_register(disk, &bi); } -- cgit v1.2.3 From 3a2d1efaf344c7351945532020774c3fcb134a0a Mon Sep 17 00:00:00 2001 From: Deepak R Varma Date: Sun, 8 Jan 2023 21:58:51 +0530 Subject: scsi: ipr: Make ipr_probe_ioa_part2() return void Convert function ipr_probe_ioa_part2() to return void instead of int since the current implementation always returns 0 to the caller. The transformation also eliminates the dead code when calling ipr_probe_ioa_part2() function. Issue identified using returnvar Coccinelle semantic patch. Link: https://lore.kernel.org/r/Y7rvQyMOGcPKPTv8@ubun2204.myguest.virtualbox.org Signed-off-by: Deepak R Varma Acked-by: Brian King Signed-off-by: Martin K. Petersen --- drivers/scsi/ipr.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c index 7dff517a0858..5feffda0ef68 100644 --- a/drivers/scsi/ipr.c +++ b/drivers/scsi/ipr.c @@ -9500,11 +9500,10 @@ static pci_ers_result_t ipr_pci_error_detected(struct pci_dev *pdev, * This function takes care of initilizing the adapter to the point * where it can accept new commands. * Return value: - * 0 on success / -EIO on failure + * none **/ -static int ipr_probe_ioa_part2(struct ipr_ioa_cfg *ioa_cfg) +static void ipr_probe_ioa_part2(struct ipr_ioa_cfg *ioa_cfg) { - int rc = 0; unsigned long host_lock_flags = 0; ENTER; @@ -9520,7 +9519,6 @@ static int ipr_probe_ioa_part2(struct ipr_ioa_cfg *ioa_cfg) spin_unlock_irqrestore(ioa_cfg->host->host_lock, host_lock_flags); LEAVE; - return rc; } /** @@ -10563,12 +10561,7 @@ static int ipr_probe(struct pci_dev *pdev, const struct pci_device_id *dev_id) return rc; ioa_cfg = pci_get_drvdata(pdev); - rc = ipr_probe_ioa_part2(ioa_cfg); - - if (rc) { - __ipr_remove(pdev); - return rc; - } + ipr_probe_ioa_part2(ioa_cfg); rc = scsi_add_host(ioa_cfg->host, &pdev->dev); -- cgit v1.2.3 From ee4e7dfe4ffc9ca50c6875757bd119abfe22b5c5 Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Tue, 14 Feb 2023 14:28:08 +0100 Subject: scsi: ipr: Work around fortify-string warning The ipr_log_vpd_compact() function triggers a fortified memcpy() warning about a potential string overflow with all versions of clang: In file included from drivers/scsi/ipr.c:43: In file included from include/linux/string.h:254: include/linux/fortify-string.h:520:4: error: call to '__write_overflow_field' declared with 'warning' attribute: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror,-Wattribute-warning] __write_overflow_field(p_size_field, size); ^ include/linux/fortify-string.h:520:4: error: call to '__write_overflow_field' declared with 'warning' attribute: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror,-Wattribute-warning] 2 errors generated. I don't see anything actually wrong with the function, but this is the only instance I can reproduce of the fortification going wrong in the kernel at the moment, so the easiest solution may be to rewrite the function into something that does not trigger the warning. Instead of having a combined buffer for vendor/device/serial strings, use three separate local variables and just truncate the whitespace individually. Link: https://lore.kernel.org/r/20230214132831.2118392-1-arnd@kernel.org Cc: Kees Cook Fixes: 8cf093e275d0 ("[SCSI] ipr: Improved dual adapter errors") Signed-off-by: Arnd Bergmann Reviewed-by: Damien Le Moal Reviewed-by: Kees Cook Acked-by: Brian King Signed-off-by: Martin K. Petersen --- drivers/scsi/ipr.c | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c index 5feffda0ef68..b9f6a16cc1c6 100644 --- a/drivers/scsi/ipr.c +++ b/drivers/scsi/ipr.c @@ -1516,23 +1516,22 @@ static void ipr_process_ccn(struct ipr_cmnd *ipr_cmd) } /** - * strip_and_pad_whitespace - Strip and pad trailing whitespace. - * @i: index into buffer - * @buf: string to modify + * strip_whitespace - Strip and pad trailing whitespace. + * @i: size of buffer + * @buf: string to modify * - * This function will strip all trailing whitespace, pad the end - * of the string with a single space, and NULL terminate the string. + * This function will strip all trailing whitespace and + * NUL terminate the string. * - * Return value: - * new length of string **/ -static int strip_and_pad_whitespace(int i, char *buf) +static void strip_whitespace(int i, char *buf) { + if (i < 1) + return; + i--; while (i && buf[i] == ' ') i--; - buf[i+1] = ' '; - buf[i+2] = '\0'; - return i + 2; + buf[i+1] = '\0'; } /** @@ -1547,19 +1546,21 @@ static int strip_and_pad_whitespace(int i, char *buf) static void ipr_log_vpd_compact(char *prefix, struct ipr_hostrcb *hostrcb, struct ipr_vpd *vpd) { - char buffer[IPR_VENDOR_ID_LEN + IPR_PROD_ID_LEN + IPR_SERIAL_NUM_LEN + 3]; - int i = 0; + char vendor_id[IPR_VENDOR_ID_LEN + 1]; + char product_id[IPR_PROD_ID_LEN + 1]; + char sn[IPR_SERIAL_NUM_LEN + 1]; - memcpy(buffer, vpd->vpids.vendor_id, IPR_VENDOR_ID_LEN); - i = strip_and_pad_whitespace(IPR_VENDOR_ID_LEN - 1, buffer); + memcpy(vendor_id, vpd->vpids.vendor_id, IPR_VENDOR_ID_LEN); + strip_whitespace(IPR_VENDOR_ID_LEN, vendor_id); - memcpy(&buffer[i], vpd->vpids.product_id, IPR_PROD_ID_LEN); - i = strip_and_pad_whitespace(i + IPR_PROD_ID_LEN - 1, buffer); + memcpy(product_id, vpd->vpids.product_id, IPR_PROD_ID_LEN); + strip_whitespace(IPR_PROD_ID_LEN, product_id); - memcpy(&buffer[i], vpd->sn, IPR_SERIAL_NUM_LEN); - buffer[IPR_SERIAL_NUM_LEN + i] = '\0'; + memcpy(sn, vpd->sn, IPR_SERIAL_NUM_LEN); + strip_whitespace(IPR_SERIAL_NUM_LEN, sn); - ipr_hcam_err(hostrcb, "%s VPID/SN: %s\n", prefix, buffer); + ipr_hcam_err(hostrcb, "%s VPID/SN: %s %s %s\n", prefix, + vendor_id, product_id, sn); } /** -- cgit v1.2.3 From c6f2e6b6eaaf883df482cb94f302acad9b80a2a4 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Sat, 4 Feb 2023 10:37:16 -0800 Subject: scsi: mpi3mr: Replace 1-element array with flex-array Nothing else defined MPI3_NVME_ENCAP_CMD_MAX, so the "command" buffer was being defined as a fake flexible array of size 1. Replace this with a proper flex array. Avoids this GCC 13 warning under -fstrict-flex-arrays=3: In function 'fortify_memset_chk', inlined from 'mpi3mr_build_nvme_sgl' at ../drivers/scsi/mpi3mr/mpi3mr_app.c:693:2, inlined from 'mpi3mr_bsg_process_mpt_cmds.constprop' at ../drivers/scsi/mpi3mr/mpi3mr_app.c:1214:8: ../include/linux/fortify-string.h:430:25: warning: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Wattribute-warning] 430 | __write_overflow_field(p_size_field, size); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Link: https://lore.kernel.org/r/20230204183715.never.937-kees@kernel.org Cc: Sumit Saxena Cc: Himanshu Madhani Cc: "Martin K. Petersen" Cc: Stephen Rothwell Cc: kernel test robot Signed-off-by: Kees Cook Signed-off-by: Martin K. Petersen --- include/uapi/scsi/scsi_bsg_mpi3mr.h | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/include/uapi/scsi/scsi_bsg_mpi3mr.h b/include/uapi/scsi/scsi_bsg_mpi3mr.h index fdc3517f9e19..907d345f04f9 100644 --- a/include/uapi/scsi/scsi_bsg_mpi3mr.h +++ b/include/uapi/scsi/scsi_bsg_mpi3mr.h @@ -455,12 +455,6 @@ struct mpi3mr_bsg_packet { } cmd; }; - -/* MPI3: NVMe Encasulation related definitions */ -#ifndef MPI3_NVME_ENCAP_CMD_MAX -#define MPI3_NVME_ENCAP_CMD_MAX (1) -#endif - struct mpi3_nvme_encapsulated_request { __le16 host_tag; __u8 ioc_use_only02; @@ -474,7 +468,7 @@ struct mpi3_nvme_encapsulated_request { __le16 flags; __le32 data_length; __le32 reserved14[3]; - __le32 command[MPI3_NVME_ENCAP_CMD_MAX]; + __le32 command[]; }; struct mpi3_nvme_encapsulated_error_reply { -- cgit v1.2.3 From ae7d45f5283d30274039b95d3e6d53d33c66e991 Mon Sep 17 00:00:00 2001 From: Tomas Henzl Date: Mon, 13 Feb 2023 20:37:52 +0100 Subject: scsi: mpi3mr: Fix an issue found by KASAN Write only correct size (32 instead of 64 bytes). Link: https://lore.kernel.org/r/20230213193752.6859-1-thenzl@redhat.com Fixes: 42fc9fee116f ("scsi: mpi3mr: Add helper functions to manage device's port") Signed-off-by: Tomas Henzl Acked-by: Sathya Prakash Veerichetty Signed-off-by: Martin K. Petersen --- drivers/scsi/mpi3mr/mpi3mr_transport.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/mpi3mr/mpi3mr_transport.c b/drivers/scsi/mpi3mr/mpi3mr_transport.c index 3fc897336b5e..3b61815979da 100644 --- a/drivers/scsi/mpi3mr/mpi3mr_transport.c +++ b/drivers/scsi/mpi3mr/mpi3mr_transport.c @@ -1280,7 +1280,7 @@ void mpi3mr_sas_host_add(struct mpi3mr_ioc *mrioc) if (mrioc->sas_hba.enclosure_handle) { if (!(mpi3mr_cfg_get_enclosure_pg0(mrioc, &ioc_status, - &encl_pg0, sizeof(dev_pg0), + &encl_pg0, sizeof(encl_pg0), MPI3_ENCLOS_PGAD_FORM_HANDLE, mrioc->sas_hba.enclosure_handle)) && (ioc_status == MPI3_IOCSTATUS_SUCCESS)) -- cgit v1.2.3 From fb428a2005fc1260d18b989cc5199f281617f44d Mon Sep 17 00:00:00 2001 From: Shin'ichiro Kawasaki Date: Tue, 14 Feb 2023 09:50:16 +0900 Subject: scsi: mpi3mr: Fix issues in mpi3mr_get_all_tgt_info() The function mpi3mr_get_all_tgt_info() has four issues: 1) It calculates valid entry length in alltgt_info assuming the header part of the struct mpi3mr_device_map_info would equal to sizeof(u32). The correct size is sizeof(u64). 2) When it calculates the valid entry length kern_entrylen, it excludes one entry by subtracting 1 from num_devices. 3) It copies num_device by calling memcpy(). Substitution is enough. 4) It does not specify the calculated length to sg_copy_from_buffer(). Instead, it specifies the payload length which is larger than the alltgt_info size. It causes "BUG: KASAN: slab-out-of-bounds". Fix the issues by using the correct header size, removing the subtraction from num_devices, replacing the memcpy() with substitution and specifying the correct length to sg_copy_from_buffer(). Link: https://lore.kernel.org/r/20230214005019.1897251-2-shinichiro.kawasaki@wdc.com Cc: stable@vger.kernel.org Fixes: f5e6d5a34376 ("scsi: mpi3mr: Add support for driver commands") Signed-off-by: Shin'ichiro Kawasaki Acked-by: Sathya Prakash Veerichetty Signed-off-by: Martin K. Petersen --- drivers/scsi/mpi3mr/mpi3mr_app.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/drivers/scsi/mpi3mr/mpi3mr_app.c b/drivers/scsi/mpi3mr/mpi3mr_app.c index 9baac224b213..72054e3a26cb 100644 --- a/drivers/scsi/mpi3mr/mpi3mr_app.c +++ b/drivers/scsi/mpi3mr/mpi3mr_app.c @@ -312,7 +312,7 @@ static long mpi3mr_get_all_tgt_info(struct mpi3mr_ioc *mrioc, num_devices++; spin_unlock_irqrestore(&mrioc->tgtdev_lock, flags); - if ((job->request_payload.payload_len == sizeof(u32)) || + if ((job->request_payload.payload_len <= sizeof(u64)) || list_empty(&mrioc->tgtdev_list)) { sg_copy_from_buffer(job->request_payload.sg_list, job->request_payload.sg_cnt, @@ -320,14 +320,14 @@ static long mpi3mr_get_all_tgt_info(struct mpi3mr_ioc *mrioc, return 0; } - kern_entrylen = (num_devices - 1) * sizeof(*devmap_info); - size = sizeof(*alltgt_info) + kern_entrylen; + kern_entrylen = num_devices * sizeof(*devmap_info); + size = sizeof(u64) + kern_entrylen; alltgt_info = kzalloc(size, GFP_KERNEL); if (!alltgt_info) return -ENOMEM; devmap_info = alltgt_info->dmi; - memset((u8 *)devmap_info, 0xFF, (kern_entrylen + sizeof(*devmap_info))); + memset((u8 *)devmap_info, 0xFF, kern_entrylen); spin_lock_irqsave(&mrioc->tgtdev_lock, flags); list_for_each_entry(tgtdev, &mrioc->tgtdev_list, list) { if (i < num_devices) { @@ -344,9 +344,10 @@ static long mpi3mr_get_all_tgt_info(struct mpi3mr_ioc *mrioc, num_devices = i; spin_unlock_irqrestore(&mrioc->tgtdev_lock, flags); - memcpy(&alltgt_info->num_devices, &num_devices, sizeof(num_devices)); + alltgt_info->num_devices = num_devices; - usr_entrylen = (job->request_payload.payload_len - sizeof(u32)) / sizeof(*devmap_info); + usr_entrylen = (job->request_payload.payload_len - sizeof(u64)) / + sizeof(*devmap_info); usr_entrylen *= sizeof(*devmap_info); min_entrylen = min(usr_entrylen, kern_entrylen); if (min_entrylen && (!memcpy(&alltgt_info->dmi, devmap_info, min_entrylen))) { @@ -358,7 +359,7 @@ static long mpi3mr_get_all_tgt_info(struct mpi3mr_ioc *mrioc, sg_copy_from_buffer(job->request_payload.sg_list, job->request_payload.sg_cnt, - alltgt_info, job->request_payload.payload_len); + alltgt_info, (min_entrylen + sizeof(u64))); rval = 0; out: kfree(alltgt_info); -- cgit v1.2.3 From eeb270aee3e085411399f129fc14fa04bd6d83cf Mon Sep 17 00:00:00 2001 From: Shin'ichiro Kawasaki Date: Tue, 14 Feb 2023 09:50:17 +0900 Subject: scsi: mpi3mr: Remove unnecessary memcpy() to alltgt_info->dmi In the function mpi3mr_get_all_tgt_info(), devmap_info points to alltgt_info->dmi then there is no need to memcpy() data from devmap_info to alltgt_info->dmi. Remove the unnecessary memcpy(). This also allows to remove the local variable 'rval' and the goto label 'out'. Link: https://lore.kernel.org/r/20230214005019.1897251-3-shinichiro.kawasaki@wdc.com Cc: stable@vger.kernel.org Fixes: f5e6d5a34376 ("scsi: mpi3mr: Add support for driver commands") Signed-off-by: Shin'ichiro Kawasaki Acked-by: Sathya Prakash Veerichetty Signed-off-by: Martin K. Petersen --- drivers/scsi/mpi3mr/mpi3mr_app.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/drivers/scsi/mpi3mr/mpi3mr_app.c b/drivers/scsi/mpi3mr/mpi3mr_app.c index 72054e3a26cb..bff637702397 100644 --- a/drivers/scsi/mpi3mr/mpi3mr_app.c +++ b/drivers/scsi/mpi3mr/mpi3mr_app.c @@ -293,7 +293,6 @@ out: static long mpi3mr_get_all_tgt_info(struct mpi3mr_ioc *mrioc, struct bsg_job *job) { - long rval = -EINVAL; u16 num_devices = 0, i = 0, size; unsigned long flags; struct mpi3mr_tgt_dev *tgtdev; @@ -304,7 +303,7 @@ static long mpi3mr_get_all_tgt_info(struct mpi3mr_ioc *mrioc, if (job->request_payload.payload_len < sizeof(u32)) { dprint_bsg_err(mrioc, "%s: invalid size argument\n", __func__); - return rval; + return -EINVAL; } spin_lock_irqsave(&mrioc->tgtdev_lock, flags); @@ -350,20 +349,12 @@ static long mpi3mr_get_all_tgt_info(struct mpi3mr_ioc *mrioc, sizeof(*devmap_info); usr_entrylen *= sizeof(*devmap_info); min_entrylen = min(usr_entrylen, kern_entrylen); - if (min_entrylen && (!memcpy(&alltgt_info->dmi, devmap_info, min_entrylen))) { - dprint_bsg_err(mrioc, "%s:%d: device map info copy failed\n", - __func__, __LINE__); - rval = -EFAULT; - goto out; - } sg_copy_from_buffer(job->request_payload.sg_list, job->request_payload.sg_cnt, alltgt_info, (min_entrylen + sizeof(u64))); - rval = 0; -out: kfree(alltgt_info); - return rval; + return 0; } /** * mpi3mr_get_change_count - Get topology change count -- cgit v1.2.3 From 339e61565f81a6534afdc18fd854b2e2628bf5db Mon Sep 17 00:00:00 2001 From: Shin'ichiro Kawasaki Date: Tue, 14 Feb 2023 09:50:18 +0900 Subject: scsi: mpi3mr: Use number of bits to manage bitmap sizes To allocate bitmaps, the mpi3mr driver calculates sizes of bitmaps using byte as unit. However, bitmap helper functions assume that bitmaps are allocated using unsigned long as unit. This gap causes memory access beyond the bitmap sizes and results in "BUG: KASAN: slab-out-of-bounds". The BUG was observed at firmware download to eHBA-9600. Call trace indicated that the out-of-bounds access happened in find_first_zero_bit() called from mpi3mr_send_event_ack() for miroc->evtack_cmds_bitmap. To fix the BUG, do not use bytes to manage bitmap sizes. Instead, use number of bits, and call bitmap helper functions which take number of bits as arguments. For memory allocation, call bitmap_zalloc() instead of kzalloc() and krealloc(). For memory free, call bitmap_free() instead of kfree(). For zero clear, call bitmap_clear() instead of memset(). Remove three fields for bitmap byte sizes in struct scmd_priv which are no longer required. Replace the field dev_handle_bitmap_sz with dev_handle_bitmap_bits to keep number of bits of removepend_bitmap across resize. Link: https://lore.kernel.org/r/20230214005019.1897251-4-shinichiro.kawasaki@wdc.com Fixes: c5758fc72b92 ("scsi: mpi3mr: Gracefully handle online FW update operation") Fixes: e844adb1fbdc ("scsi: mpi3mr: Implement SCSI error handler hooks") Fixes: c1af985d27da ("scsi: mpi3mr: Add Event acknowledgment logic") Fixes: 824a156633df ("scsi: mpi3mr: Base driver code") Signed-off-by: Shin'ichiro Kawasaki Acked-by: Sathya Prakash Veerichetty Signed-off-by: Martin K. Petersen --- drivers/scsi/mpi3mr/mpi3mr.h | 10 ++---- drivers/scsi/mpi3mr/mpi3mr_fw.c | 75 +++++++++++++++++------------------------ 2 files changed, 33 insertions(+), 52 deletions(-) diff --git a/drivers/scsi/mpi3mr/mpi3mr.h b/drivers/scsi/mpi3mr/mpi3mr.h index def4c5e15cd8..8a438f248a82 100644 --- a/drivers/scsi/mpi3mr/mpi3mr.h +++ b/drivers/scsi/mpi3mr/mpi3mr.h @@ -955,19 +955,16 @@ struct scmd_priv { * @chain_buf_count: Chain buffer count * @chain_buf_pool: Chain buffer pool * @chain_sgl_list: Chain SGL list - * @chain_bitmap_sz: Chain buffer allocator bitmap size * @chain_bitmap: Chain buffer allocator bitmap * @chain_buf_lock: Chain buffer list lock * @bsg_cmds: Command tracker for BSG command * @host_tm_cmds: Command tracker for task management commands * @dev_rmhs_cmds: Command tracker for device removal commands * @evtack_cmds: Command tracker for event ack commands - * @devrem_bitmap_sz: Device removal bitmap size * @devrem_bitmap: Device removal bitmap - * @dev_handle_bitmap_sz: Device handle bitmap size + * @dev_handle_bitmap_bits: Number of bits in device handle bitmap * @removepend_bitmap: Remove pending bitmap * @delayed_rmhs_list: Delayed device removal list - * @evtack_cmds_bitmap_sz: Event Ack bitmap size * @evtack_cmds_bitmap: Event Ack bitmap * @delayed_evtack_cmds_list: Delayed event acknowledgment list * @ts_update_counter: Timestamp update counter @@ -1128,7 +1125,6 @@ struct mpi3mr_ioc { u32 chain_buf_count; struct dma_pool *chain_buf_pool; struct chain_element *chain_sgl_list; - u16 chain_bitmap_sz; void *chain_bitmap; spinlock_t chain_buf_lock; @@ -1136,12 +1132,10 @@ struct mpi3mr_ioc { struct mpi3mr_drv_cmd host_tm_cmds; struct mpi3mr_drv_cmd dev_rmhs_cmds[MPI3MR_NUM_DEVRMCMD]; struct mpi3mr_drv_cmd evtack_cmds[MPI3MR_NUM_EVTACKCMD]; - u16 devrem_bitmap_sz; void *devrem_bitmap; - u16 dev_handle_bitmap_sz; + u16 dev_handle_bitmap_bits; void *removepend_bitmap; struct list_head delayed_rmhs_list; - u16 evtack_cmds_bitmap_sz; void *evtack_cmds_bitmap; struct list_head delayed_evtack_cmds_list; diff --git a/drivers/scsi/mpi3mr/mpi3mr_fw.c b/drivers/scsi/mpi3mr/mpi3mr_fw.c index 0c4aabaefdcc..1e4467ea8472 100644 --- a/drivers/scsi/mpi3mr/mpi3mr_fw.c +++ b/drivers/scsi/mpi3mr/mpi3mr_fw.c @@ -1128,7 +1128,6 @@ static int mpi3mr_issue_and_process_mur(struct mpi3mr_ioc *mrioc, static int mpi3mr_revalidate_factsdata(struct mpi3mr_ioc *mrioc) { - u16 dev_handle_bitmap_sz; void *removepend_bitmap; if (mrioc->facts.reply_sz > mrioc->reply_sz) { @@ -1160,25 +1159,23 @@ mpi3mr_revalidate_factsdata(struct mpi3mr_ioc *mrioc) "\tcontroller while sas transport support is enabled at the\n" "\tdriver, please reboot the system or reload the driver\n"); - dev_handle_bitmap_sz = mrioc->facts.max_devhandle / 8; - if (mrioc->facts.max_devhandle % 8) - dev_handle_bitmap_sz++; - if (dev_handle_bitmap_sz > mrioc->dev_handle_bitmap_sz) { - removepend_bitmap = krealloc(mrioc->removepend_bitmap, - dev_handle_bitmap_sz, GFP_KERNEL); + if (mrioc->facts.max_devhandle > mrioc->dev_handle_bitmap_bits) { + removepend_bitmap = bitmap_zalloc(mrioc->facts.max_devhandle, + GFP_KERNEL); if (!removepend_bitmap) { ioc_err(mrioc, - "failed to increase removepend_bitmap sz from: %d to %d\n", - mrioc->dev_handle_bitmap_sz, dev_handle_bitmap_sz); + "failed to increase removepend_bitmap bits from %d to %d\n", + mrioc->dev_handle_bitmap_bits, + mrioc->facts.max_devhandle); return -EPERM; } - memset(removepend_bitmap + mrioc->dev_handle_bitmap_sz, 0, - dev_handle_bitmap_sz - mrioc->dev_handle_bitmap_sz); + bitmap_free(mrioc->removepend_bitmap); mrioc->removepend_bitmap = removepend_bitmap; ioc_info(mrioc, - "increased dev_handle_bitmap_sz from %d to %d\n", - mrioc->dev_handle_bitmap_sz, dev_handle_bitmap_sz); - mrioc->dev_handle_bitmap_sz = dev_handle_bitmap_sz; + "increased bits of dev_handle_bitmap from %d to %d\n", + mrioc->dev_handle_bitmap_bits, + mrioc->facts.max_devhandle); + mrioc->dev_handle_bitmap_bits = mrioc->facts.max_devhandle; } return 0; @@ -2957,27 +2954,18 @@ static int mpi3mr_alloc_reply_sense_bufs(struct mpi3mr_ioc *mrioc) if (!mrioc->pel_abort_cmd.reply) goto out_failed; - mrioc->dev_handle_bitmap_sz = mrioc->facts.max_devhandle / 8; - if (mrioc->facts.max_devhandle % 8) - mrioc->dev_handle_bitmap_sz++; - mrioc->removepend_bitmap = kzalloc(mrioc->dev_handle_bitmap_sz, - GFP_KERNEL); + mrioc->dev_handle_bitmap_bits = mrioc->facts.max_devhandle; + mrioc->removepend_bitmap = bitmap_zalloc(mrioc->dev_handle_bitmap_bits, + GFP_KERNEL); if (!mrioc->removepend_bitmap) goto out_failed; - mrioc->devrem_bitmap_sz = MPI3MR_NUM_DEVRMCMD / 8; - if (MPI3MR_NUM_DEVRMCMD % 8) - mrioc->devrem_bitmap_sz++; - mrioc->devrem_bitmap = kzalloc(mrioc->devrem_bitmap_sz, - GFP_KERNEL); + mrioc->devrem_bitmap = bitmap_zalloc(MPI3MR_NUM_DEVRMCMD, GFP_KERNEL); if (!mrioc->devrem_bitmap) goto out_failed; - mrioc->evtack_cmds_bitmap_sz = MPI3MR_NUM_EVTACKCMD / 8; - if (MPI3MR_NUM_EVTACKCMD % 8) - mrioc->evtack_cmds_bitmap_sz++; - mrioc->evtack_cmds_bitmap = kzalloc(mrioc->evtack_cmds_bitmap_sz, - GFP_KERNEL); + mrioc->evtack_cmds_bitmap = bitmap_zalloc(MPI3MR_NUM_EVTACKCMD, + GFP_KERNEL); if (!mrioc->evtack_cmds_bitmap) goto out_failed; @@ -3415,10 +3403,7 @@ static int mpi3mr_alloc_chain_bufs(struct mpi3mr_ioc *mrioc) if (!mrioc->chain_sgl_list[i].addr) goto out_failed; } - mrioc->chain_bitmap_sz = num_chains / 8; - if (num_chains % 8) - mrioc->chain_bitmap_sz++; - mrioc->chain_bitmap = kzalloc(mrioc->chain_bitmap_sz, GFP_KERNEL); + mrioc->chain_bitmap = bitmap_zalloc(num_chains, GFP_KERNEL); if (!mrioc->chain_bitmap) goto out_failed; return retval; @@ -4190,10 +4175,11 @@ void mpi3mr_memset_buffers(struct mpi3mr_ioc *mrioc) for (i = 0; i < MPI3MR_NUM_EVTACKCMD; i++) memset(mrioc->evtack_cmds[i].reply, 0, sizeof(*mrioc->evtack_cmds[i].reply)); - memset(mrioc->removepend_bitmap, 0, mrioc->dev_handle_bitmap_sz); - memset(mrioc->devrem_bitmap, 0, mrioc->devrem_bitmap_sz); - memset(mrioc->evtack_cmds_bitmap, 0, - mrioc->evtack_cmds_bitmap_sz); + bitmap_clear(mrioc->removepend_bitmap, 0, + mrioc->dev_handle_bitmap_bits); + bitmap_clear(mrioc->devrem_bitmap, 0, MPI3MR_NUM_DEVRMCMD); + bitmap_clear(mrioc->evtack_cmds_bitmap, 0, + MPI3MR_NUM_EVTACKCMD); } for (i = 0; i < mrioc->num_queues; i++) { @@ -4319,16 +4305,16 @@ void mpi3mr_free_mem(struct mpi3mr_ioc *mrioc) mrioc->evtack_cmds[i].reply = NULL; } - kfree(mrioc->removepend_bitmap); + bitmap_free(mrioc->removepend_bitmap); mrioc->removepend_bitmap = NULL; - kfree(mrioc->devrem_bitmap); + bitmap_free(mrioc->devrem_bitmap); mrioc->devrem_bitmap = NULL; - kfree(mrioc->evtack_cmds_bitmap); + bitmap_free(mrioc->evtack_cmds_bitmap); mrioc->evtack_cmds_bitmap = NULL; - kfree(mrioc->chain_bitmap); + bitmap_free(mrioc->chain_bitmap); mrioc->chain_bitmap = NULL; kfree(mrioc->transport_cmds.reply); @@ -4887,9 +4873,10 @@ int mpi3mr_soft_reset_handler(struct mpi3mr_ioc *mrioc, mpi3mr_flush_delayed_cmd_lists(mrioc); mpi3mr_flush_drv_cmds(mrioc); - memset(mrioc->devrem_bitmap, 0, mrioc->devrem_bitmap_sz); - memset(mrioc->removepend_bitmap, 0, mrioc->dev_handle_bitmap_sz); - memset(mrioc->evtack_cmds_bitmap, 0, mrioc->evtack_cmds_bitmap_sz); + bitmap_clear(mrioc->devrem_bitmap, 0, MPI3MR_NUM_DEVRMCMD); + bitmap_clear(mrioc->removepend_bitmap, 0, + mrioc->dev_handle_bitmap_bits); + bitmap_clear(mrioc->evtack_cmds_bitmap, 0, MPI3MR_NUM_EVTACKCMD); mpi3mr_flush_host_io(mrioc); mpi3mr_cleanup_fwevt_list(mrioc); mpi3mr_invalidate_devhandles(mrioc); -- cgit v1.2.3 From e39ea831ebad4ab15c4748cb62a397a8abcca36e Mon Sep 17 00:00:00 2001 From: Shin'ichiro Kawasaki Date: Tue, 14 Feb 2023 09:50:19 +0900 Subject: scsi: mpi3mr: Fix missing mrioc->evtack_cmds initialization Commit c1af985d27da ("scsi: mpi3mr: Add Event acknowledgment logic") introduced an array mrioc->evtack_cmds but initialization of the array elements was missed. They are just zero cleared. The function mpi3mr_complete_evt_ack() refers host_tag field of the elements. Due to the zero value of the host_tag field, the function calls clear_bit() for mrico->evtack_cmds_bitmap with wrong bit index. This results in memory access to invalid address and "BUG: KASAN: use-after-free". This BUG was observed at eHBA-9600 firmware update to version 8.3.1.0. To fix it, add the missing initialization of mrioc->evtack_cmds. Link: https://lore.kernel.org/r/20230214005019.1897251-5-shinichiro.kawasaki@wdc.com Cc: stable@vger.kernel.org Fixes: c1af985d27da ("scsi: mpi3mr: Add Event acknowledgment logic") Signed-off-by: Shin'ichiro Kawasaki Reviewed-by: Damien Le Moal Acked-by: Sathya Prakash Veerichetty Signed-off-by: Martin K. Petersen --- drivers/scsi/mpi3mr/mpi3mr_os.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c index 3306de7170f6..6eaeba41072c 100644 --- a/drivers/scsi/mpi3mr/mpi3mr_os.c +++ b/drivers/scsi/mpi3mr/mpi3mr_os.c @@ -4952,6 +4952,10 @@ mpi3mr_probe(struct pci_dev *pdev, const struct pci_device_id *id) mpi3mr_init_drv_cmd(&mrioc->dev_rmhs_cmds[i], MPI3MR_HOSTTAG_DEVRMCMD_MIN + i); + for (i = 0; i < MPI3MR_NUM_EVTACKCMD; i++) + mpi3mr_init_drv_cmd(&mrioc->evtack_cmds[i], + MPI3MR_HOSTTAG_EVTACKCMD_MIN + i); + if (pdev->revision) mrioc->enable_segqueue = true; -- cgit v1.2.3 From 66b381d874fa440dbcd20e1a9078849f7dd2ff4c Mon Sep 17 00:00:00 2001 From: Jesper Juhl Date: Thu, 16 Feb 2023 01:51:30 +0100 Subject: scsi: mpi3mr: Remove unneeded version.h include Remove unneeded version.h include pointed out by 'make versioncheck'. Link: https://lore.kernel.org/r/820137c2-decc-3d78-f170-7f1c0571fbb7@gmail.com Signed-off-by: Jesper Juhl Signed-off-by: Martin K. Petersen --- drivers/scsi/mpi3mr/mpi3mr.h | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/scsi/mpi3mr/mpi3mr.h b/drivers/scsi/mpi3mr/mpi3mr.h index 8a438f248a82..23de2603e71f 100644 --- a/drivers/scsi/mpi3mr/mpi3mr.h +++ b/drivers/scsi/mpi3mr/mpi3mr.h @@ -29,7 +29,6 @@ #include #include #include -#include #include #include #include -- cgit v1.2.3 From e0aa38c444f2fcf1fceb4fef7902209dd25b48ea Mon Sep 17 00:00:00 2001 From: Jesper Juhl Date: Thu, 16 Feb 2023 01:51:30 +0100 Subject: scsi: qedi: Remove unneeded version.h include Remove unneeded version.h include pointed out by 'make versioncheck'. Link: https://lore.kernel.org/r/820137c2-decc-3d78-f170-7f1c0571fbb7@gmail.com Signed-off-by: Jesper Juhl Signed-off-by: Martin K. Petersen --- drivers/scsi/qedi/qedi_dbg.h | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/scsi/qedi/qedi_dbg.h b/drivers/scsi/qedi/qedi_dbg.h index 37d084086fd4..fdda12ef13b0 100644 --- a/drivers/scsi/qedi/qedi_dbg.h +++ b/drivers/scsi/qedi/qedi_dbg.h @@ -11,7 +11,6 @@ #include #include #include -#include #include #include #include -- cgit v1.2.3 From 5794a3902bde986ade963cdf4db98b6baa433d20 Mon Sep 17 00:00:00 2001 From: Jesper Juhl Date: Thu, 16 Feb 2023 01:51:30 +0100 Subject: scsi: cxgbi: Remove unneeded version.h include Remove unneeded version.h include pointed out by 'make versioncheck'. Link: https://lore.kernel.org/r/820137c2-decc-3d78-f170-7f1c0571fbb7@gmail.com Signed-off-by: Jesper Juhl Signed-off-by: Martin K. Petersen --- drivers/scsi/cxgbi/libcxgbi.h | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/scsi/cxgbi/libcxgbi.h b/drivers/scsi/cxgbi/libcxgbi.h index 3687b5c0cf90..d8fc7beafa20 100644 --- a/drivers/scsi/cxgbi/libcxgbi.h +++ b/drivers/scsi/cxgbi/libcxgbi.h @@ -24,7 +24,6 @@ #include #include #include -#include #include #include -- cgit v1.2.3 From f4407e6033a69ff633ee9d6acf6d241223c7c9fb Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Fri, 10 Feb 2023 12:51:59 -0800 Subject: scsi: core: Fix a source code comment Fix an incorrect reference to the scsi_remove_host() function in a source code comment. Link: https://lore.kernel.org/r/20230210205200.36973-2-bvanassche@acm.org Fixes: b49493f99690 ("Fix a memory leak in scsi_host_dev_release()") Signed-off-by: Bart Van Assche Signed-off-by: Martin K. Petersen --- drivers/scsi/hosts.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c index 12346e2297fd..b28375f9e019 100644 --- a/drivers/scsi/hosts.c +++ b/drivers/scsi/hosts.c @@ -356,7 +356,7 @@ static void scsi_host_dev_release(struct device *dev) /* * Free the shost_dev device name here if scsi_host_alloc() * and scsi_host_put() have been called but neither - * scsi_host_add() nor scsi_host_remove() has been called. + * scsi_host_add() nor scsi_remove_host() has been called. * This avoids that the memory allocated for the shost_dev * name is leaked. */ -- cgit v1.2.3 From fc663711b94468f4e1427ebe289c9f05669699c9 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Fri, 10 Feb 2023 12:52:00 -0800 Subject: scsi: core: Remove the /proc/scsi/${proc_name} directory earlier Remove the /proc/scsi/${proc_name} directory earlier to fix a race condition between unloading and reloading kernel modules. This fixes a bug introduced in 2009 by commit 77c019768f06 ("[SCSI] fix /proc memory leak in the SCSI core"). Fix the following kernel warning: proc_dir_entry 'scsi/scsi_debug' already registered WARNING: CPU: 19 PID: 27986 at fs/proc/generic.c:376 proc_register+0x27d/0x2e0 Call Trace: proc_mkdir+0xb5/0xe0 scsi_proc_hostdir_add+0xb5/0x170 scsi_host_alloc+0x683/0x6c0 sdebug_driver_probe+0x6b/0x2d0 [scsi_debug] really_probe+0x159/0x540 __driver_probe_device+0xdc/0x230 driver_probe_device+0x4f/0x120 __device_attach_driver+0xef/0x180 bus_for_each_drv+0xe5/0x130 __device_attach+0x127/0x290 device_initial_probe+0x17/0x20 bus_probe_device+0x110/0x130 device_add+0x673/0xc80 device_register+0x1e/0x30 sdebug_add_host_helper+0x1a7/0x3b0 [scsi_debug] scsi_debug_init+0x64f/0x1000 [scsi_debug] do_one_initcall+0xd7/0x470 do_init_module+0xe7/0x330 load_module+0x122a/0x12c0 __do_sys_finit_module+0x124/0x1a0 __x64_sys_finit_module+0x46/0x50 do_syscall_64+0x38/0x80 entry_SYSCALL_64_after_hwframe+0x46/0xb0 Link: https://lore.kernel.org/r/20230210205200.36973-3-bvanassche@acm.org Cc: Alan Stern Cc: Yi Zhang Cc: stable@vger.kernel.org Fixes: 77c019768f06 ("[SCSI] fix /proc memory leak in the SCSI core") Reported-by: Yi Zhang Signed-off-by: Bart Van Assche Signed-off-by: Martin K. Petersen --- drivers/scsi/hosts.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c index b28375f9e019..f7f62e56afca 100644 --- a/drivers/scsi/hosts.c +++ b/drivers/scsi/hosts.c @@ -181,6 +181,7 @@ void scsi_remove_host(struct Scsi_Host *shost) scsi_forget_host(shost); mutex_unlock(&shost->scan_mutex); scsi_proc_host_rm(shost); + scsi_proc_hostdir_rm(shost->hostt); /* * New SCSI devices cannot be attached anymore because of the SCSI host @@ -340,6 +341,7 @@ static void scsi_host_dev_release(struct device *dev) struct Scsi_Host *shost = dev_to_shost(dev); struct device *parent = dev->parent; + /* In case scsi_remove_host() has not been called. */ scsi_proc_hostdir_rm(shost->hostt); /* Wait for functions invoked through call_rcu(&scmd->rcu, ...) */ -- cgit v1.2.3 From 442336a5a999d4aae6221e3496dad071a152b1fb Mon Sep 17 00:00:00 2001 From: Bo Liu Date: Fri, 17 Feb 2023 03:30:46 -0500 Subject: scsi: lpfc: Fix double word in comments Remove the repeated word "the" in comments. [mkp: fixed additional typos in the changed lines] Link: https://lore.kernel.org/r/20230217083046.4090-1-liubo03@inspur.com Signed-off-by: Bo Liu Reviewed-by: Randy Dunlap Signed-off-by: Martin K. Petersen --- drivers/scsi/lpfc/lpfc_attr.c | 10 +++++----- drivers/scsi/lpfc/lpfc_els.c | 2 +- drivers/scsi/lpfc/lpfc_hbadisc.c | 2 +- drivers/scsi/lpfc/lpfc_init.c | 4 ++-- drivers/scsi/lpfc/lpfc_mbox.c | 4 ++-- drivers/scsi/lpfc/lpfc_nvmet.c | 2 +- drivers/scsi/lpfc/lpfc_sli.c | 2 +- 7 files changed, 13 insertions(+), 13 deletions(-) diff --git a/drivers/scsi/lpfc/lpfc_attr.c b/drivers/scsi/lpfc/lpfc_attr.c index 76c3434f8976..22f2e046e8eb 100644 --- a/drivers/scsi/lpfc/lpfc_attr.c +++ b/drivers/scsi/lpfc/lpfc_attr.c @@ -2541,7 +2541,7 @@ lpfc_sriov_hw_max_virtfn_show(struct device *dev, /** * lpfc_enable_bbcr_set: Sets an attribute value. - * @phba: pointer the the adapter structure. + * @phba: pointer to the adapter structure. * @val: integer attribute value. * * Description: @@ -2632,7 +2632,7 @@ lpfc_##attr##_show(struct device *dev, struct device_attribute *attr, \ * takes a default argument, a minimum and maximum argument. * * lpfc_##attr##_init: Initializes an attribute. - * @phba: pointer the the adapter structure. + * @phba: pointer to the adapter structure. * @val: integer attribute value. * * Validates the min and max values then sets the adapter config field @@ -2665,7 +2665,7 @@ lpfc_##attr##_init(struct lpfc_hba *phba, uint val) \ * into a function with the name lpfc_hba_queue_depth_set * * lpfc_##attr##_set: Sets an attribute value. - * @phba: pointer the the adapter structure. + * @phba: pointer to the adapter structure. * @val: integer attribute value. * * Description: @@ -2794,7 +2794,7 @@ lpfc_##attr##_show(struct device *dev, struct device_attribute *attr, \ * lpfc_##attr##_init: validates the min and max values then sets the * adapter config field accordingly, or uses the default if out of range * and prints an error message. - * @phba: pointer the the adapter structure. + * @phba: pointer to the adapter structure. * @val: integer attribute value. * * Returns: @@ -2826,7 +2826,7 @@ lpfc_##attr##_init(struct lpfc_vport *vport, uint val) \ * lpfc_##attr##_set: validates the min and max values then sets the * adapter config field if in the valid range. prints error message * and does not set the parameter if invalid. - * @phba: pointer the the adapter structure. + * @phba: pointer to the adapter structure. * @val: integer attribute value. * * Returns: diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c index aee5d0d1187d..35b252f1ef73 100644 --- a/drivers/scsi/lpfc/lpfc_els.c +++ b/drivers/scsi/lpfc/lpfc_els.c @@ -8886,7 +8886,7 @@ reject_out: * @rrq: Pointer to the rrq struct. * * Build a ELS RRQ command and send it to the target. If the issue_iocb is - * Successful the the completion handler will clear the RRQ. + * successful, the completion handler will clear the RRQ. * * Return codes * 0 - Successfully sent rrq els iocb. diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c index a6df0a5b4006..66cd0b1dbbd0 100644 --- a/drivers/scsi/lpfc/lpfc_hbadisc.c +++ b/drivers/scsi/lpfc/lpfc_hbadisc.c @@ -2459,7 +2459,7 @@ static void lpfc_sli4_fcf_pri_list_del(struct lpfc_hba *phba, * @phba: pointer to lpfc hba data structure. * @fcf_index: the index of the fcf record to update * This routine acquires the hbalock and then set the LPFC_FCF_FLOGI_FAILED - * flag so the the round robin slection for the particular priority level + * flag so the round robin selection for the particular priority level * will try a different fcf record that does not have this bit set. * If the fcf record is re-read for any reason this flag is cleared brfore * adding it to the priority list. diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c index faaaeae25d44..75737088d011 100644 --- a/drivers/scsi/lpfc/lpfc_init.c +++ b/drivers/scsi/lpfc/lpfc_init.c @@ -5501,7 +5501,7 @@ lpfc_sli4_async_link_evt(struct lpfc_hba *phba, bf_set(lpfc_mbx_read_top_link_spd, la, (bf_get(lpfc_acqe_link_speed, acqe_link))); - /* Fake the the following irrelvant fields */ + /* Fake the following irrelevant fields */ bf_set(lpfc_mbx_read_top_topology, la, LPFC_TOPOLOGY_PT_PT); bf_set(lpfc_mbx_read_top_alpa_granted, la, 0); bf_set(lpfc_mbx_read_top_il, la, 0); @@ -12548,7 +12548,7 @@ lpfc_cpu_affinity_check(struct lpfc_hba *phba, int vectors) /* Mark CPU as IRQ not assigned by the kernel */ cpup->flag |= LPFC_CPU_MAP_UNASSIGN; - /* If so, find a new_cpup thats on the the SAME + /* If so, find a new_cpup that is on the SAME * phys_id as cpup. start_cpu will start where we * left off so all unassigned entries don't get assgined * the IRQ of the first entry. diff --git a/drivers/scsi/lpfc/lpfc_mbox.c b/drivers/scsi/lpfc/lpfc_mbox.c index 9858b1743769..0dfdc0c4c08c 100644 --- a/drivers/scsi/lpfc/lpfc_mbox.c +++ b/drivers/scsi/lpfc/lpfc_mbox.c @@ -2509,7 +2509,7 @@ lpfc_sli4_dump_page_a0(struct lpfc_hba *phba, struct lpfcMboxq *mbox) * information via a READ_FCF mailbox command. This mailbox command also is used * to indicate where received unsolicited frames from this FCF will be sent. By * default this routine will set up the FCF to forward all unsolicited frames - * the the RQ ID passed in the @phba. This can be overridden by the caller for + * to the RQ ID passed in the @phba. This can be overridden by the caller for * more complicated setups. **/ void @@ -2577,7 +2577,7 @@ lpfc_reg_fcfi(struct lpfc_hba *phba, struct lpfcMboxq *mbox) * information via a READ_FCF mailbox command. This mailbox command also is used * to indicate where received unsolicited frames from this FCF will be sent. By * default this routine will set up the FCF to forward all unsolicited frames - * the the RQ ID passed in the @phba. This can be overridden by the caller for + * to the RQ ID passed in the @phba. This can be overridden by the caller for * more complicated setups. **/ void diff --git a/drivers/scsi/lpfc/lpfc_nvmet.c b/drivers/scsi/lpfc/lpfc_nvmet.c index f7cfac0da9b6..7517dd55fe91 100644 --- a/drivers/scsi/lpfc/lpfc_nvmet.c +++ b/drivers/scsi/lpfc/lpfc_nvmet.c @@ -1469,7 +1469,7 @@ lpfc_nvmet_cleanup_io_context(struct lpfc_hba *phba) if (!infop) return; - /* Cycle the the entire CPU context list for every MRQ */ + /* Cycle the entire CPU context list for every MRQ */ for (i = 0; i < phba->cfg_nvmet_mrq; i++) { for_each_present_cpu(j) { infop = lpfc_get_ctx_list(phba, j, i); diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c index edbd81c3b643..c5b69f313af3 100644 --- a/drivers/scsi/lpfc/lpfc_sli.c +++ b/drivers/scsi/lpfc/lpfc_sli.c @@ -20804,7 +20804,7 @@ lpfc_log_fw_write_cmpl(struct lpfc_hba *phba, u32 shdr_status, * the offset after the write object mailbox has completed. @size is used to * determine the end of the object and whether the eof bit should be set. * - * Return 0 is successful and offset will contain the the new offset to use + * Return 0 is successful and offset will contain the new offset to use * for the next write. * Return negative value for error cases. **/ -- cgit v1.2.3 From 35cd2f5542df569122d48caf606b972642012c50 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Fri, 10 Feb 2023 11:32:56 -0800 Subject: scsi: core: Extend struct scsi_exec_args Allow SCSI LLDs to specify SCMD_* flags. Link: https://lore.kernel.org/r/20230210193258.4004923-2-bvanassche@acm.org Cc: Mike Christie Cc: John Garry Reviewed-by: John Garry Signed-off-by: Bart Van Assche Signed-off-by: Martin K. Petersen --- drivers/scsi/scsi_lib.c | 1 + include/scsi/scsi_device.h | 1 + 2 files changed, 2 insertions(+) diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index abe93ec8b7d0..b7c569a42aa4 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -229,6 +229,7 @@ int scsi_execute_cmd(struct scsi_device *sdev, const unsigned char *cmd, scmd->cmd_len = COMMAND_SIZE(cmd[0]); memcpy(scmd->cmnd, cmd, scmd->cmd_len); scmd->allowed = retries; + scmd->flags |= args->scmd_flags; req->timeout = timeout; req->rq_flags |= RQF_QUIET; diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h index 7e95ec45138f..de310f21406c 100644 --- a/include/scsi/scsi_device.h +++ b/include/scsi/scsi_device.h @@ -462,6 +462,7 @@ struct scsi_exec_args { unsigned int sense_len; /* sense buffer len */ struct scsi_sense_hdr *sshdr; /* decoded sense header */ blk_mq_req_flags_t req_flags; /* BLK_MQ_REQ flags */ + int scmd_flags; /* SCMD flags */ int *resid; /* residual length */ }; -- cgit v1.2.3 From 93bc4a5d00e472003ae983bb21febd2519a64f62 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Fri, 10 Feb 2023 11:32:57 -0800 Subject: scsi: ufs: core: Rely on the block layer for setting RQF_PM Do not set RQF_PM explicitly since scsi_alloc_request() sets it indirectly if BLK_MQ_REQ_PM is set. The call chain for the code that sets RQF_PM is as follows: scsi_alloc_request() blk_mq_alloc_request() __blk_mq_alloc_requests() blk_mq_rq_ctx_init() if (data->flags & BLK_MQ_REQ_PM) data->rq_flags |= RQF_PM; Link: https://lore.kernel.org/r/20230210193258.4004923-3-bvanassche@acm.org Cc: Mike Christie Cc: John Garry Reviewed-by: John Garry Signed-off-by: Bart Van Assche Signed-off-by: Martin K. Petersen --- drivers/ufs/core/ufshcd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index 47c7739b9a86..9d8e03b30014 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -9150,7 +9150,7 @@ static int ufshcd_execute_start_stop(struct scsi_device *sdev, scmd->allowed = 0/*retries*/; scmd->flags |= SCMD_FAIL_IF_RECOVERING; req->timeout = 1 * HZ; - req->rq_flags |= RQF_PM | RQF_QUIET; + req->rq_flags |= RQF_QUIET; blk_execute_rq(req, /*at_head=*/true); -- cgit v1.2.3 From 2702812ae33b38898de6d950cdb6a03888d001af Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Fri, 10 Feb 2023 11:32:58 -0800 Subject: scsi: ufs: core: Simplify ufshcd_execute_start_stop() Use scsi_execute_cmd() instead of open-coding it. Link: https://lore.kernel.org/r/20230210193258.4004923-4-bvanassche@acm.org Cc: Mike Christie Cc: John Garry Signed-off-by: Bart Van Assche Reviewed-by: John Garry Reviewed-by: Asutosh Das Signed-off-by: Martin K. Petersen --- drivers/ufs/core/ufshcd.c | 35 ++++++++--------------------------- 1 file changed, 8 insertions(+), 27 deletions(-) diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index 9d8e03b30014..629442c6bc9c 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -9134,34 +9134,15 @@ static int ufshcd_execute_start_stop(struct scsi_device *sdev, enum ufs_dev_pwr_mode pwr_mode, struct scsi_sense_hdr *sshdr) { - unsigned char cdb[6] = { START_STOP, 0, 0, 0, pwr_mode << 4, 0 }; - struct request *req; - struct scsi_cmnd *scmd; - int ret; - - req = scsi_alloc_request(sdev->request_queue, REQ_OP_DRV_IN, - BLK_MQ_REQ_PM); - if (IS_ERR(req)) - return PTR_ERR(req); - - scmd = blk_mq_rq_to_pdu(req); - scmd->cmd_len = COMMAND_SIZE(cdb[0]); - memcpy(scmd->cmnd, cdb, scmd->cmd_len); - scmd->allowed = 0/*retries*/; - scmd->flags |= SCMD_FAIL_IF_RECOVERING; - req->timeout = 1 * HZ; - req->rq_flags |= RQF_QUIET; - - blk_execute_rq(req, /*at_head=*/true); - - if (sshdr) - scsi_normalize_sense(scmd->sense_buffer, scmd->sense_len, - sshdr); - ret = scmd->result; - - blk_mq_free_request(req); + const unsigned char cdb[6] = { START_STOP, 0, 0, 0, pwr_mode << 4, 0 }; + const struct scsi_exec_args args = { + .sshdr = sshdr, + .req_flags = BLK_MQ_REQ_PM, + .scmd_flags = SCMD_FAIL_IF_RECOVERING, + }; - return ret; + return scsi_execute_cmd(sdev, cdb, REQ_OP_DRV_IN, /*buffer=*/NULL, + /*bufflen=*/0, /*timeout=*/HZ, /*retries=*/0, &args); } /** -- cgit v1.2.3 From 79f9abd64719cc71ba78a76574e21dc8266c65a3 Mon Sep 17 00:00:00 2001 From: Benjamin Block Date: Tue, 21 Feb 2023 18:55:58 +0100 Subject: scsi: zfcp: Make the type for accessing request hashtable buckets size_t The appropriate type for array indices is 'size_t' and the current implementation in 'zfcp_reqlist.h' mixes 'int' and 'unsigned int' in different places to access the hashtable buckets of our internal request hash table. To prevent any confusion, change all places to 'size_t'. Link: https://lore.kernel.org/r/64afe93f6263c6b07815937826cd7d5fc4f1a674.1677000450.git.bblock@linux.ibm.com Signed-off-by: Benjamin Block Reviewed-by: Steffen Maier Signed-off-by: Martin K. Petersen --- drivers/s390/scsi/zfcp_reqlist.h | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/drivers/s390/scsi/zfcp_reqlist.h b/drivers/s390/scsi/zfcp_reqlist.h index 9b8ff249e31c..f4bac61dfbd0 100644 --- a/drivers/s390/scsi/zfcp_reqlist.h +++ b/drivers/s390/scsi/zfcp_reqlist.h @@ -5,14 +5,16 @@ * Data structure and helper functions for tracking pending FSF * requests. * - * Copyright IBM Corp. 2009, 2016 + * Copyright IBM Corp. 2009, 2023 */ #ifndef ZFCP_REQLIST_H #define ZFCP_REQLIST_H +#include + /* number of hash buckets */ -#define ZFCP_REQ_LIST_BUCKETS 128 +#define ZFCP_REQ_LIST_BUCKETS 128u /** * struct zfcp_reqlist - Container for request list (reqlist) @@ -24,7 +26,7 @@ struct zfcp_reqlist { struct list_head buckets[ZFCP_REQ_LIST_BUCKETS]; }; -static inline int zfcp_reqlist_hash(unsigned long req_id) +static inline size_t zfcp_reqlist_hash(unsigned long req_id) { return req_id % ZFCP_REQ_LIST_BUCKETS; } @@ -37,7 +39,7 @@ static inline int zfcp_reqlist_hash(unsigned long req_id) */ static inline struct zfcp_reqlist *zfcp_reqlist_alloc(void) { - unsigned int i; + size_t i; struct zfcp_reqlist *rl; rl = kzalloc(sizeof(struct zfcp_reqlist), GFP_KERNEL); @@ -60,7 +62,7 @@ static inline struct zfcp_reqlist *zfcp_reqlist_alloc(void) */ static inline int zfcp_reqlist_isempty(struct zfcp_reqlist *rl) { - unsigned int i; + size_t i; for (i = 0; i < ZFCP_REQ_LIST_BUCKETS; i++) if (!list_empty(&rl->buckets[i])) @@ -84,7 +86,7 @@ static inline struct zfcp_fsf_req * _zfcp_reqlist_find(struct zfcp_reqlist *rl, unsigned long req_id) { struct zfcp_fsf_req *req; - unsigned int i; + size_t i; i = zfcp_reqlist_hash(req_id); list_for_each_entry(req, &rl->buckets[i], list) @@ -154,7 +156,7 @@ zfcp_reqlist_find_rm(struct zfcp_reqlist *rl, unsigned long req_id) static inline void zfcp_reqlist_add(struct zfcp_reqlist *rl, struct zfcp_fsf_req *req) { - unsigned int i; + size_t i; unsigned long flags; i = zfcp_reqlist_hash(req->req_id); @@ -172,7 +174,7 @@ static inline void zfcp_reqlist_add(struct zfcp_reqlist *rl, static inline void zfcp_reqlist_move(struct zfcp_reqlist *rl, struct list_head *list) { - unsigned int i; + size_t i; unsigned long flags; spin_lock_irqsave(&rl->lock, flags); @@ -200,7 +202,7 @@ zfcp_reqlist_apply_for_all(struct zfcp_reqlist *rl, { struct zfcp_fsf_req *req; unsigned long flags; - unsigned int i; + size_t i; spin_lock_irqsave(&rl->lock, flags); for (i = 0; i < ZFCP_REQ_LIST_BUCKETS; i++) -- cgit v1.2.3 From 3ab01810153b0ca26cfc846ba72996b250adde6d Mon Sep 17 00:00:00 2001 From: Benjamin Block Date: Tue, 21 Feb 2023 18:55:59 +0100 Subject: scsi: zfcp: Change the type of all fsf request id fields and variables to u64 We use different integer types throughout zfcp to store the FSF request ID and related values; some places use 'unsigned long' and others 'u64'. On s390x these are effectively the same type, but this might cause confusions and is generally inconsistent. The specification for the used hardware specifies this value as a 64-bit number, and ultimately we use this value to communicate with the hardware, so it makes sense to change the type of all these variables to 'u64' where we can. The only exception being when we store it in the 'host_scribble' field of a 'struct scsi_cmnd'; for this case we add a build time check to make sure they are compatible. Link: https://lore.kernel.org/r/9c9cbe5acc2b419a22dce2fed847e3db91b60201.1677000450.git.bblock@linux.ibm.com Signed-off-by: Benjamin Block Reviewed-by: Steffen Maier Signed-off-by: Martin K. Petersen --- drivers/s390/scsi/zfcp_dbf.c | 2 +- drivers/s390/scsi/zfcp_def.h | 6 +++--- drivers/s390/scsi/zfcp_fsf.c | 15 ++++++++------- drivers/s390/scsi/zfcp_qdio.h | 2 +- drivers/s390/scsi/zfcp_reqlist.h | 8 ++++---- drivers/s390/scsi/zfcp_scsi.c | 2 +- 6 files changed, 18 insertions(+), 17 deletions(-) diff --git a/drivers/s390/scsi/zfcp_dbf.c b/drivers/s390/scsi/zfcp_dbf.c index cbc3b62cd9e5..d28c55ae9f01 100644 --- a/drivers/s390/scsi/zfcp_dbf.c +++ b/drivers/s390/scsi/zfcp_dbf.c @@ -649,7 +649,7 @@ void zfcp_dbf_scsi_common(char *tag, int level, struct scsi_device *sdev, rec->scsi_id = sc->device->id; rec->scsi_lun = (u32)sc->device->lun; rec->scsi_lun_64_hi = (u32)(sc->device->lun >> 32); - rec->host_scribble = (unsigned long)sc->host_scribble; + rec->host_scribble = (u64)sc->host_scribble; memcpy(rec->scsi_opcode, sc->cmnd, min_t(int, sc->cmd_len, ZFCP_DBF_SCSI_OPCODE)); diff --git a/drivers/s390/scsi/zfcp_def.h b/drivers/s390/scsi/zfcp_def.h index 94de55304a02..6c761299a22f 100644 --- a/drivers/s390/scsi/zfcp_def.h +++ b/drivers/s390/scsi/zfcp_def.h @@ -129,7 +129,7 @@ struct zfcp_erp_action { struct scsi_device *sdev; u32 status; /* recovery status */ enum zfcp_erp_steps step; /* active step of this erp action */ - unsigned long fsf_req_id; + u64 fsf_req_id; struct timer_list timer; }; @@ -163,7 +163,7 @@ struct zfcp_adapter { struct Scsi_Host *scsi_host; /* Pointer to mid-layer */ struct list_head port_list; /* remote port list */ rwlock_t port_list_lock; /* port list lock */ - unsigned long req_no; /* unique FSF req number */ + u64 req_no; /* unique FSF req number */ struct zfcp_reqlist *req_list; u32 fsf_req_seq_no; /* FSF cmnd seq number */ rwlock_t abort_lock; /* Protects against SCSI @@ -325,7 +325,7 @@ static inline u64 zfcp_scsi_dev_lun(struct scsi_device *sdev) */ struct zfcp_fsf_req { struct list_head list; - unsigned long req_id; + u64 req_id; struct zfcp_adapter *adapter; struct zfcp_qdio_req qdio_req; struct completion completion; diff --git a/drivers/s390/scsi/zfcp_fsf.c b/drivers/s390/scsi/zfcp_fsf.c index ab3ea529cca7..71eabcc26cbb 100644 --- a/drivers/s390/scsi/zfcp_fsf.c +++ b/drivers/s390/scsi/zfcp_fsf.c @@ -884,7 +884,7 @@ static int zfcp_fsf_req_send(struct zfcp_fsf_req *req) const bool is_srb = zfcp_fsf_req_is_status_read_buffer(req); struct zfcp_adapter *adapter = req->adapter; struct zfcp_qdio *qdio = adapter->qdio; - unsigned long req_id = req->req_id; + u64 req_id = req->req_id; zfcp_reqlist_add(adapter->req_list, req); @@ -1042,7 +1042,7 @@ struct zfcp_fsf_req *zfcp_fsf_abort_fcp_cmnd(struct scsi_cmnd *scmnd) struct scsi_device *sdev = scmnd->device; struct zfcp_scsi_dev *zfcp_sdev = sdev_to_zfcp(sdev); struct zfcp_qdio *qdio = zfcp_sdev->port->adapter->qdio; - unsigned long old_req_id = (unsigned long) scmnd->host_scribble; + u64 old_req_id = (u64) scmnd->host_scribble; spin_lock_irq(&qdio->req_q_lock); if (zfcp_qdio_sbal_get(qdio)) @@ -1065,7 +1065,7 @@ struct zfcp_fsf_req *zfcp_fsf_abort_fcp_cmnd(struct scsi_cmnd *scmnd) req->handler = zfcp_fsf_abort_fcp_command_handler; req->qtcb->header.lun_handle = zfcp_sdev->lun_handle; req->qtcb->header.port_handle = zfcp_sdev->port->handle; - req->qtcb->bottom.support.req_handle = (u64) old_req_id; + req->qtcb->bottom.support.req_handle = old_req_id; zfcp_fsf_start_timer(req, ZFCP_FSF_SCSI_ER_TIMEOUT); if (!zfcp_fsf_req_send(req)) { @@ -1919,7 +1919,7 @@ int zfcp_fsf_open_wka_port(struct zfcp_fc_wka_port *wka_port) { struct zfcp_qdio *qdio = wka_port->adapter->qdio; struct zfcp_fsf_req *req; - unsigned long req_id = 0; + u64 req_id = 0; int retval = -EIO; spin_lock_irq(&qdio->req_q_lock); @@ -1978,7 +1978,7 @@ int zfcp_fsf_close_wka_port(struct zfcp_fc_wka_port *wka_port) { struct zfcp_qdio *qdio = wka_port->adapter->qdio; struct zfcp_fsf_req *req; - unsigned long req_id = 0; + u64 req_id = 0; int retval = -EIO; spin_lock_irq(&qdio->req_q_lock); @@ -2587,6 +2587,7 @@ int zfcp_fsf_fcp_cmnd(struct scsi_cmnd *scsi_cmnd) goto out; } + BUILD_BUG_ON(sizeof(scsi_cmnd->host_scribble) < sizeof(req->req_id)); scsi_cmnd->host_scribble = (unsigned char *) req->req_id; io = &req->qtcb->bottom.io; @@ -2732,7 +2733,7 @@ void zfcp_fsf_reqid_check(struct zfcp_qdio *qdio, int sbal_idx) struct qdio_buffer *sbal = qdio->res_q[sbal_idx]; struct qdio_buffer_element *sbale; struct zfcp_fsf_req *fsf_req; - unsigned long req_id; + u64 req_id; int idx; for (idx = 0; idx < QDIO_MAX_ELEMENTS_PER_BUFFER; idx++) { @@ -2747,7 +2748,7 @@ void zfcp_fsf_reqid_check(struct zfcp_qdio *qdio, int sbal_idx) * corruption and must stop the machine immediately. */ zfcp_qdio_siosl(adapter); - panic("error: unknown req_id (%lx) on adapter %s.\n", + panic("error: unknown req_id (%llx) on adapter %s.\n", req_id, dev_name(&adapter->ccw_device->dev)); } diff --git a/drivers/s390/scsi/zfcp_qdio.h b/drivers/s390/scsi/zfcp_qdio.h index 390706867df3..90134d9b69a7 100644 --- a/drivers/s390/scsi/zfcp_qdio.h +++ b/drivers/s390/scsi/zfcp_qdio.h @@ -115,7 +115,7 @@ zfcp_qdio_sbale_curr(struct zfcp_qdio *qdio, struct zfcp_qdio_req *q_req) */ static inline void zfcp_qdio_req_init(struct zfcp_qdio *qdio, struct zfcp_qdio_req *q_req, - unsigned long req_id, u8 sbtype, void *data, u32 len) + u64 req_id, u8 sbtype, void *data, u32 len) { struct qdio_buffer_element *sbale; int count = min(atomic_read(&qdio->req_q_free), diff --git a/drivers/s390/scsi/zfcp_reqlist.h b/drivers/s390/scsi/zfcp_reqlist.h index f4bac61dfbd0..59fbb1b128cb 100644 --- a/drivers/s390/scsi/zfcp_reqlist.h +++ b/drivers/s390/scsi/zfcp_reqlist.h @@ -26,7 +26,7 @@ struct zfcp_reqlist { struct list_head buckets[ZFCP_REQ_LIST_BUCKETS]; }; -static inline size_t zfcp_reqlist_hash(unsigned long req_id) +static inline size_t zfcp_reqlist_hash(u64 req_id) { return req_id % ZFCP_REQ_LIST_BUCKETS; } @@ -83,7 +83,7 @@ static inline void zfcp_reqlist_free(struct zfcp_reqlist *rl) } static inline struct zfcp_fsf_req * -_zfcp_reqlist_find(struct zfcp_reqlist *rl, unsigned long req_id) +_zfcp_reqlist_find(struct zfcp_reqlist *rl, u64 req_id) { struct zfcp_fsf_req *req; size_t i; @@ -104,7 +104,7 @@ _zfcp_reqlist_find(struct zfcp_reqlist *rl, unsigned long req_id) * or NULL if there is no known FSF request with this id. */ static inline struct zfcp_fsf_req * -zfcp_reqlist_find(struct zfcp_reqlist *rl, unsigned long req_id) +zfcp_reqlist_find(struct zfcp_reqlist *rl, u64 req_id) { unsigned long flags; struct zfcp_fsf_req *req; @@ -129,7 +129,7 @@ zfcp_reqlist_find(struct zfcp_reqlist *rl, unsigned long req_id) * NULL if it has not been found. */ static inline struct zfcp_fsf_req * -zfcp_reqlist_find_rm(struct zfcp_reqlist *rl, unsigned long req_id) +zfcp_reqlist_find_rm(struct zfcp_reqlist *rl, u64 req_id) { unsigned long flags; struct zfcp_fsf_req *req; diff --git a/drivers/s390/scsi/zfcp_scsi.c b/drivers/s390/scsi/zfcp_scsi.c index 526ac240d9fe..3dbf4b21d127 100644 --- a/drivers/s390/scsi/zfcp_scsi.c +++ b/drivers/s390/scsi/zfcp_scsi.c @@ -170,7 +170,7 @@ static int zfcp_scsi_eh_abort_handler(struct scsi_cmnd *scpnt) (struct zfcp_adapter *) scsi_host->hostdata[0]; struct zfcp_fsf_req *old_req, *abrt_req; unsigned long flags; - unsigned long old_reqid = (unsigned long) scpnt->host_scribble; + u64 old_reqid = (u64) scpnt->host_scribble; int retval = SUCCESS, ret; int retry = 3; char *dbf_tag; -- cgit v1.2.3 From 901b894af5b933cf6576eec05746f34b46e2ac83 Mon Sep 17 00:00:00 2001 From: Benjamin Block Date: Tue, 21 Feb 2023 18:56:00 +0100 Subject: scsi: zfcp: Trace when request remove fails after qdio send fails When we fail to send a FSF request in 'zfcp_fsf_req_send()' when calling 'zfcp_qdio_send()' we try to remove the request object from our internal hash table again to prevent keeping a stale memory reference. This removal might still - very much theoretically - fail. To store some evidence of when this happens add a new trace record for this case; tag: 'fsrsrmf'. We reuse the 'ZFCP_DBF_HBA_RES' trace ID for this, but mark all fields other then the request ID with ~0, to make fairly obvious that these are invalid values. This faking has to be done because we don't have a valid request object at this point, and can not safely access any of the memory of the old object - we just failed to find it in our hash table, so it might be gone already. Here is an example of a decoded trace record: Timestamp : 2023-02-17-13:09:12:748140 Area : HBA Subarea : 1 Level : - Exception : 000003ff7ff500c2 CPU ID : 0011 Caller : 0x0 Record ID : 1 Tag : fsrsrmf Request ID : 0x0000000080126ab6 Request status : 0xffffffff FSF cmnd : 0xffffffff FSF sequence no: 0xffffffff FSF issued : 2042-09-18-01:53:47:370495 FSF stat : 0xffffffff FSF stat qual : ffffffff ffffffff ffffffff ffffffff Prot stat : 0xffffffff Prot stat qual : ffffffff ffffffff ffffffff ffffffff Port handle : 0xffffffff LUN handle : 0xffffffff This provides at least some basic evidence that this event happened, and what object was affected. Link: https://lore.kernel.org/r/99b8246b2d71b63fa4f9c56333e2037502f7f5af.1677000450.git.bblock@linux.ibm.com Signed-off-by: Benjamin Block Reviewed-by: Steffen Maier Signed-off-by: Martin K. Petersen --- drivers/s390/scsi/zfcp_dbf.c | 44 +++++++++++++++++++++++++++++++++++++++++++- drivers/s390/scsi/zfcp_ext.h | 5 ++++- drivers/s390/scsi/zfcp_fsf.c | 7 +++++-- 3 files changed, 52 insertions(+), 4 deletions(-) diff --git a/drivers/s390/scsi/zfcp_dbf.c b/drivers/s390/scsi/zfcp_dbf.c index d28c55ae9f01..d904625afd40 100644 --- a/drivers/s390/scsi/zfcp_dbf.c +++ b/drivers/s390/scsi/zfcp_dbf.c @@ -4,7 +4,7 @@ * * Debug traces for zfcp. * - * Copyright IBM Corp. 2002, 2020 + * Copyright IBM Corp. 2002, 2023 */ #define KMSG_COMPONENT "zfcp" @@ -145,6 +145,48 @@ void zfcp_dbf_hba_fsf_fces(char *tag, const struct zfcp_fsf_req *req, u64 wwpn, spin_unlock_irqrestore(&dbf->hba_lock, flags); } +/** + * zfcp_dbf_hba_fsf_reqid - trace only the tag and a request ID + * @tag: tag documenting the source + * @level: trace level + * @adapter: adapter instance the request ID belongs to + * @req_id: the request ID to trace + */ +void zfcp_dbf_hba_fsf_reqid(const char *const tag, const int level, + struct zfcp_adapter *const adapter, + const u64 req_id) +{ + struct zfcp_dbf *const dbf = adapter->dbf; + struct zfcp_dbf_hba *const rec = &dbf->hba_buf; + struct zfcp_dbf_hba_res *const res = &rec->u.res; + unsigned long flags; + + if (unlikely(!debug_level_enabled(dbf->hba, level))) + return; + + spin_lock_irqsave(&dbf->hba_lock, flags); + memset(rec, 0, sizeof(*rec)); + + memcpy(rec->tag, tag, ZFCP_DBF_TAG_LEN); + + rec->id = ZFCP_DBF_HBA_RES; + rec->fsf_req_id = req_id; + rec->fsf_req_status = ~0u; + rec->fsf_cmd = ~0u; + rec->fsf_seq_no = ~0u; + + res->req_issued = ~0ull; + res->prot_status = ~0u; + memset(res->prot_status_qual, 0xff, sizeof(res->prot_status_qual)); + res->fsf_status = ~0u; + memset(res->fsf_status_qual, 0xff, sizeof(res->fsf_status_qual)); + res->port_handle = ~0u; + res->lun_handle = ~0u; + + debug_event(dbf->hba, level, rec, sizeof(*rec)); + spin_unlock_irqrestore(&dbf->hba_lock, flags); +} + /** * zfcp_dbf_hba_fsf_uss - trace event for an unsolicited status buffer * @tag: tag indicating which kind of unsolicited status has been received diff --git a/drivers/s390/scsi/zfcp_ext.h b/drivers/s390/scsi/zfcp_ext.h index c302cbb18a55..9f5152b42b0e 100644 --- a/drivers/s390/scsi/zfcp_ext.h +++ b/drivers/s390/scsi/zfcp_ext.h @@ -4,7 +4,7 @@ * * External function declarations. * - * Copyright IBM Corp. 2002, 2020 + * Copyright IBM Corp. 2002, 2023 */ #ifndef ZFCP_EXT_H @@ -46,6 +46,9 @@ extern void zfcp_dbf_hba_fsf_res(char *, int, struct zfcp_fsf_req *); extern void zfcp_dbf_hba_fsf_fces(char *tag, const struct zfcp_fsf_req *req, u64 wwpn, u32 fc_security_old, u32 fc_security_new); +extern void zfcp_dbf_hba_fsf_reqid(const char *const tag, const int level, + struct zfcp_adapter *const adapter, + const u64 req_id); extern void zfcp_dbf_hba_bit_err(char *, struct zfcp_fsf_req *); extern void zfcp_dbf_hba_def_err(struct zfcp_adapter *, u64, u16, void **); extern void zfcp_dbf_san_req(char *, struct zfcp_fsf_req *, u32); diff --git a/drivers/s390/scsi/zfcp_fsf.c b/drivers/s390/scsi/zfcp_fsf.c index 71eabcc26cbb..ceed1b6f7cb6 100644 --- a/drivers/s390/scsi/zfcp_fsf.c +++ b/drivers/s390/scsi/zfcp_fsf.c @@ -4,7 +4,7 @@ * * Implementation of FSF commands. * - * Copyright IBM Corp. 2002, 2020 + * Copyright IBM Corp. 2002, 2023 */ #define KMSG_COMPONENT "zfcp" @@ -892,8 +892,11 @@ static int zfcp_fsf_req_send(struct zfcp_fsf_req *req) req->issued = get_tod_clock(); if (zfcp_qdio_send(qdio, &req->qdio_req)) { del_timer_sync(&req->timer); + /* lookup request again, list might have changed */ - zfcp_reqlist_find_rm(adapter->req_list, req_id); + if (zfcp_reqlist_find_rm(adapter->req_list, req_id) == NULL) + zfcp_dbf_hba_fsf_reqid("fsrsrmf", 1, adapter, req_id); + zfcp_erp_adapter_reopen(adapter, 0, "fsrs__1"); return -EIO; } -- cgit v1.2.3