From patchwork Wed Apr 14 22:39:07 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Maciej W. Rozycki" X-Patchwork-Id: 12203873 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BCFB0C433ED for ; Wed, 14 Apr 2021 22:39:11 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 982C161244 for ; Wed, 14 Apr 2021 22:39:11 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234676AbhDNWjc (ORCPT ); Wed, 14 Apr 2021 18:39:32 -0400 Received: from angie.orcam.me.uk ([157.25.102.26]:38926 "EHLO angie.orcam.me.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234218AbhDNWja (ORCPT ); Wed, 14 Apr 2021 18:39:30 -0400 Received: by angie.orcam.me.uk (Postfix, from userid 500) id 3C7FC9200B3; Thu, 15 Apr 2021 00:39:07 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by angie.orcam.me.uk (Postfix) with ESMTP id 39DE492009E; Thu, 15 Apr 2021 00:39:07 +0200 (CEST) Date: Thu, 15 Apr 2021 00:39:07 +0200 (CEST) From: "Maciej W. Rozycki" To: Khalid Aziz , "James E.J. Bottomley" , "Martin K. Petersen" cc: Christoph Hellwig , linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org Subject: [PATCH 1/5] scsi: BusLogic: Fix missing `pr_cont' use In-Reply-To: Message-ID: References: User-Agent: Alpine 2.21 (DEB 202 2017-01-01) MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org Update BusLogic driver's messaging system to use `pr_cont' for continuation lines, bringing messy output: pci 0000:00:13.0: PCI->APIC IRQ transform: INT A -> IRQ 17 scsi: ***** BusLogic SCSI Driver Version 2.1.17 of 12 September 2013 ***** scsi: Copyright 1995-1998 by Leonard N. Zubkoff scsi0: Configuring BusLogic Model BT-958 PCI Wide Ultra SCSI Host Adapter scsi0: Firmware Version: 5.07B, I/O Address: 0x7000, IRQ Channel: 17/Level scsi0: PCI Bus: 0, Device: 19, Address: 0xE0012000, Host Adapter SCSI ID: 7 scsi0: Parity Checking: Enabled, Extended Translation: Enabled scsi0: Synchronous Negotiation: Ultra, Wide Negotiation: Enabled scsi0: Disconnect/Reconnect: Enabled, Tagged Queuing: Enabled scsi0: Scatter/Gather Limit: 128 of 8192 segments, Mailboxes: 211 scsi0: Driver Queue Depth: 211, Host Adapter Queue Depth: 192 scsi0: Tagged Queue Depth: Automatic , Untagged Queue Depth: 3 scsi0: SCSI Bus Termination: Both Enabled , SCAM: Disabled scsi0: *** BusLogic BT-958 Initialized Successfully *** scsi host0: BusLogic BT-958 back to order: pci 0000:00:13.0: PCI->APIC IRQ transform: INT A -> IRQ 17 scsi: ***** BusLogic SCSI Driver Version 2.1.17 of 12 September 2013 ***** scsi: Copyright 1995-1998 by Leonard N. Zubkoff scsi0: Configuring BusLogic Model BT-958 PCI Wide Ultra SCSI Host Adapter scsi0: Firmware Version: 5.07B, I/O Address: 0x7000, IRQ Channel: 17/Level scsi0: PCI Bus: 0, Device: 19, Address: 0xE0012000, Host Adapter SCSI ID: 7 scsi0: Parity Checking: Enabled, Extended Translation: Enabled scsi0: Synchronous Negotiation: Ultra, Wide Negotiation: Enabled scsi0: Disconnect/Reconnect: Enabled, Tagged Queuing: Enabled scsi0: Scatter/Gather Limit: 128 of 8192 segments, Mailboxes: 211 scsi0: Driver Queue Depth: 211, Host Adapter Queue Depth: 192 scsi0: Tagged Queue Depth: Automatic, Untagged Queue Depth: 3 scsi0: SCSI Bus Termination: Both Enabled, SCAM: Disabled scsi0: *** BusLogic BT-958 Initialized Successfully *** scsi host0: BusLogic BT-958 Also diagnostic output such as with the `BusLogic=TraceConfiguration' parameter is affected and becomes vertical and therefore hard to read. This has now been corrected, e.g.: pci 0000:00:13.0: PCI->APIC IRQ transform: INT A -> IRQ 17 blogic_cmd(86) Status = 30: 4 ==> 4: FF 05 93 00 blogic_cmd(95) Status = 28: (Modify I/O Address) blogic_cmd(91) Status = 30: 1 ==> 1: 01 blogic_cmd(04) Status = 30: 4 ==> 4: 41 41 35 30 blogic_cmd(8D) Status = 30: 14 ==> 14: 45 DC 00 20 00 00 00 00 00 40 30 37 42 1D scsi: ***** BusLogic SCSI Driver Version 2.1.17 of 12 September 2013 ***** scsi: Copyright 1995-1998 by Leonard N. Zubkoff blogic_cmd(04) Status = 30: 4 ==> 4: 41 41 35 30 blogic_cmd(0B) Status = 30: 3 ==> 3: 00 08 07 blogic_cmd(0D) Status = 30: 34 ==> 34: 03 01 07 04 00 00 00 00 00 00 00 00 00 00 00 00 FF 42 44 46 FF 00 00 00 00 00 00 00 00 00 FF 00 FF 00 blogic_cmd(8D) Status = 30: 14 ==> 14: 45 DC 00 20 00 00 00 00 00 40 30 37 42 1D blogic_cmd(84) Status = 30: 1 ==> 1: 37 blogic_cmd(8B) Status = 30: 5 ==> 5: 39 35 38 20 20 blogic_cmd(85) Status = 30: 1 ==> 1: 42 blogic_cmd(86) Status = 30: 4 ==> 4: FF 05 93 00 blogic_cmd(91) Status = 30: 64 ==> 64: 41 46 3E 20 39 35 38 20 20 00 C4 00 04 01 07 2F 07 04 35 FF FF FF FF FF FF FF FF FF FF 01 00 FE FF 08 FF FF 00 00 00 00 00 00 00 01 00 01 00 00 FF FF 00 00 00 00 00 00 00 00 00 00 00 00 00 FC scsi0: Configuring BusLogic Model BT-958 PCI Wide Ultra SCSI Host Adapter etc. Signed-off-by: Maciej W. Rozycki Fixes: 4bcc595ccd80 ("printk: reinstate KERN_CONT for printing continuation lines") Cc: stable@vger.kernel.org # v4.9+ Acked-by: Khalid Aziz --- drivers/scsi/BusLogic.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) linux-buslogic-pr-cont.diff Index: linux-macro-ide/drivers/scsi/BusLogic.c =================================================================== --- linux-macro-ide.orig/drivers/scsi/BusLogic.c +++ linux-macro-ide/drivers/scsi/BusLogic.c @@ -3603,7 +3603,7 @@ static void blogic_msg(enum blogic_msgle if (buf[0] != '\n' || len > 1) printk("%sscsi%d: %s", blogic_msglevelmap[msglevel], adapter->host_no, buf); } else - printk("%s", buf); + pr_cont("%s", buf); } else { if (begin) { if (adapter != NULL && adapter->adapter_initd) @@ -3611,7 +3611,7 @@ static void blogic_msg(enum blogic_msgle else printk("%s%s", blogic_msglevelmap[msglevel], buf); } else - printk("%s", buf); + pr_cont("%s", buf); } begin = (buf[len - 1] == '\n'); } From patchwork Wed Apr 14 22:39:12 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Maciej W. Rozycki" X-Patchwork-Id: 12203875 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1A73AC433ED for ; Wed, 14 Apr 2021 22:39:16 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E9ECD61019 for ; Wed, 14 Apr 2021 22:39:15 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234811AbhDNWjg (ORCPT ); Wed, 14 Apr 2021 18:39:36 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56790 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234218AbhDNWjf (ORCPT ); Wed, 14 Apr 2021 18:39:35 -0400 Received: from angie.orcam.me.uk (angie.orcam.me.uk [IPv6:2001:4190:8020::4]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 3252AC061574; Wed, 14 Apr 2021 15:39:13 -0700 (PDT) Received: by angie.orcam.me.uk (Postfix, from userid 500) id 9449592009E; Thu, 15 Apr 2021 00:39:12 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by angie.orcam.me.uk (Postfix) with ESMTP id 90EC192009C; Thu, 15 Apr 2021 00:39:12 +0200 (CEST) Date: Thu, 15 Apr 2021 00:39:12 +0200 (CEST) From: "Maciej W. Rozycki" To: Khalid Aziz , "James E.J. Bottomley" , "Martin K. Petersen" cc: Christoph Hellwig , linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH 2/5] scsi: BusLogic: Avoid unbounded `vsprintf' use In-Reply-To: Message-ID: References: User-Agent: Alpine 2.21 (DEB 202 2017-01-01) MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org Existing `blogic_msg' invocations do not appear to overrun its internal buffer of a fixed length of 100, which would cause stack corruption, but it's easy to miss with possible further updates and a fix is cheap in performance terms, so limit the output produced into the buffer by using `vsnprintf' rather than `vsprintf'. Signed-off-by: Maciej W. Rozycki Acked-by: Khalid Aziz --- drivers/scsi/BusLogic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) linux-buslogic-vsnprintf.diff Index: linux-macro-ide/drivers/scsi/BusLogic.c =================================================================== --- linux-macro-ide.orig/drivers/scsi/BusLogic.c +++ linux-macro-ide/drivers/scsi/BusLogic.c @@ -3588,7 +3588,7 @@ static void blogic_msg(enum blogic_msgle int len = 0; va_start(args, adapter); - len = vsprintf(buf, fmt, args); + len = vsnprintf(buf, sizeof(buf), fmt, args); va_end(args); if (msglevel == BLOGIC_ANNOUNCE_LEVEL) { static int msglines = 0; From patchwork Wed Apr 14 22:39:17 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Maciej W. Rozycki" X-Patchwork-Id: 12203877 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0856DC433B4 for ; Wed, 14 Apr 2021 22:39:27 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id CC54C611CC for ; Wed, 14 Apr 2021 22:39:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234980AbhDNWjq (ORCPT ); Wed, 14 Apr 2021 18:39:46 -0400 Received: from angie.orcam.me.uk ([157.25.102.26]:38944 "EHLO angie.orcam.me.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234702AbhDNWjm (ORCPT ); Wed, 14 Apr 2021 18:39:42 -0400 Received: by angie.orcam.me.uk (Postfix, from userid 500) id EAE9892009E; Thu, 15 Apr 2021 00:39:17 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by angie.orcam.me.uk (Postfix) with ESMTP id E7F6892009C; Thu, 15 Apr 2021 00:39:17 +0200 (CEST) Date: Thu, 15 Apr 2021 00:39:17 +0200 (CEST) From: "Maciej W. Rozycki" To: Khalid Aziz , "James E.J. Bottomley" , "Martin K. Petersen" cc: Christoph Hellwig , linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org Subject: [PATCH 3/5] scsi: Provide for avoiding trailing allocation length with VPD inquiries In-Reply-To: Message-ID: References: User-Agent: Alpine 2.21 (DEB 202 2017-01-01) MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org Allow SCSI hosts to request avoiding trailing allocation length with VPD inquiries, and use the mechanism to work around an issue with at least some BusLogic MultiMaster host bus adapters and observed with the BT-958 model specifically where issuing commands that return less data than provided for causes fatal failures: scsi host0: BusLogic BT-958 scsi 0:0:0:0: Direct-Access IBM DDYS-T18350M SA5A PQ: 0 ANSI: 3 scsi 0:0:1:0: Direct-Access SEAGATE ST336607LW 0006 PQ: 0 ANSI: 3 scsi 0:0:5:0: Direct-Access IOMEGA ZIP 100 E.08 PQ: 0 ANSI: 2 sd 0:0:1:0: [sdb] 71687372 512-byte logical blocks: (36.7 GB/34.2 GiB) sd 0:0:0:0: [sda] 35843670 512-byte logical blocks: (18.4 GB/17.1 GiB) sd 0:0:1:0: [sdb] Write Protect is off sd 0:0:5:0: [sdc] Attached SCSI removable disk sd 0:0:0:0: [sda] Write Protect is off sd 0:0:1:0: [sdb] Write cache: enabled, read cache: enabled, supports DPO and FUA sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA scsi0: *** BusLogic BT-958 Initialized Successfully *** scsi0: *** BusLogic BT-958 Initialized Successfully *** scsi0: *** BusLogic BT-958 Initialized Successfully *** scsi0: *** BusLogic BT-958 Initialized Successfully *** sd 0:0:0:0: Device offlined - not ready after error recovery sd 0:0:1:0: Device offlined - not ready after error recovery sd 0:0:0:0: scsi_vpd_inquiry(0): buf[64] => -5 sd 0:0:1:0: scsi_vpd_inquiry(0): buf[64] => -5 sd 0:0:0:0: [sda] Attached SCSI disk sd 0:0:1:0: [sdb] Attached SCSI disk VFS: Cannot open root device "802" or unknown-block(8,2): error -6 (here and elsewhere reported with some instrumentation added so as to show the causing requests and with irrelevant messages filtered out). As already observed back in 2003 and worked around in smartmontools at least some versions of BusLogic firmware such as 5.07B are unable to handle such commands, but it is possible to request enough data first for the length of the data response to be determined and then reissue the same command with the allocation length matching the response expected. It is what this change does on a host-by-host basis, by providing a flag for individual HBA drivers to enable this workaround, currently set by the BusLogic driver, and then issuing these double calls as requested, which then produce results as expected: sd 0:0:1:0: [sdb] Write cache: enabled, read cache: enabled, supports DPO and FUA sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA sd 0:0:1:0: scsi_vpd_inquiry(0): buf[64] => 13 sd 0:0:0:0: scsi_vpd_inquiry(0): buf[64] => 7 sdb: sdb1 sdb2 sd 0:0:1:0: scsi_vpd_inquiry(0): buf[64] => 13 sd 0:0:1:0: [sdb] Attached SCSI disk sda: sda1 sda2 sda3 sda4 < sda5 sda6 sda7 sda8 sda9 sda10 > sd 0:0:0:0: scsi_vpd_inquiry(0): buf[64] => 7 sd 0:0:0:0: [sda] Attached SCSI disk EXT4-fs (sda2): mounting ext2 file system using the ext4 subsystem EXT4-fs (sda2): mounted filesystem without journal. Opts: (null). Quota mode: disabled. VFS: Mounted root (ext2 filesystem) readonly on device 8:2. The minimum request size of 4 for the repeated call has been chosen to match one required for a successful return from `scsi_vpd_inquiry'. Interestingly enough it has only started triggering with not so recent commit af73623f5f10 ("[SCSI] sd: Reduce buffer size for vpd request") that decreased the allocation length for the originating request from 512 down to 64. Previously the request was rejected outright by the respective targets as invalid and therefore did not trigger the issue with MultiMaster firmware as that would only happen for a command that succeeded but produced less data than provided for: scsi0: CCB #36 Target 0: Result 2 Host Adapter Status 00 Target Status 02 scsi0: CDB 12 01 00 02 00 00 scsi0: Sense 70 00 05 00 00 00 00 18 00 00 00 00 24 00 00 C0 00 03 00 [...] sd 0:0:0:0: scsi_vpd_inquiry(0): buf[512] => -5 scsi0: CCB #37 Target 1: Result 2 Host Adapter Status 00 Target Status 02 scsi0: CDB 12 01 00 02 00 00 scsi0: Sense 70 00 05 00 00 00 00 0A 00 00 00 00 24 00 01 C9 00 03 00 [...] sd 0:0:1:0: scsi_vpd_inquiry(0): buf[512] => -5 (here with the buffer size set back to 512, the `BusLogic=TraceErrors' parameter and trailing sense data zeros trimmed for brevity). Note the sense key of 0x5 returned denoting an illegal request even for page 0. Signed-off-by: Maciej W. Rozycki Fixes: 881a256d84e6 ("[SCSI] Add VPD helper") Cc: stable@vger.kernel.org # v2.6.30+ --- drivers/scsi/BusLogic.c | 1 + drivers/scsi/scsi.c | 24 +++++++++++++++++++++--- include/scsi/scsi_host.h | 3 +++ 3 files changed, 25 insertions(+), 3 deletions(-) linux-buslogic-get-vpd-page-buffer.diff Index: linux-macro-ide/drivers/scsi/BusLogic.c =================================================================== --- linux-macro-ide.orig/drivers/scsi/BusLogic.c +++ linux-macro-ide/drivers/scsi/BusLogic.c @@ -2301,6 +2301,7 @@ static void __init blogic_inithoststruct host->sg_tablesize = adapter->drvr_sglimit; host->unchecked_isa_dma = adapter->need_bouncebuf; host->cmd_per_lun = adapter->untag_qdepth; + host->no_trailing_allocation_length = true; } /* Index: linux-macro-ide/drivers/scsi/scsi.c =================================================================== --- linux-macro-ide.orig/drivers/scsi/scsi.c +++ linux-macro-ide/drivers/scsi/scsi.c @@ -346,8 +346,19 @@ int scsi_get_vpd_page(struct scsi_device if (sdev->skip_vpd_pages) goto fail; - /* Ask for all the pages supported by this device */ - result = scsi_vpd_inquiry(sdev, buf, 0, buf_len); + /* + * Ask for all the pages supported by this device. Determine the + * actual data length first if so required by the host, e.g. + * BusLogic BT-958. + */ + if (sdev->host->no_trailing_allocation_length) { + result = scsi_vpd_inquiry(sdev, buf, 0, min(4, buf_len)); + if (result < 4) + goto fail; + } else { + result = buf_len; + } + result = scsi_vpd_inquiry(sdev, buf, 0, min(result, buf_len)); if (result < 4) goto fail; @@ -366,7 +377,14 @@ int scsi_get_vpd_page(struct scsi_device goto fail; found: - result = scsi_vpd_inquiry(sdev, buf, page, buf_len); + if (sdev->host->no_trailing_allocation_length) { + result = scsi_vpd_inquiry(sdev, buf, page, min(4, buf_len)); + if (result < 4) + goto fail; + } else { + result = buf_len; + } + result = scsi_vpd_inquiry(sdev, buf, page, min(result, buf_len)); if (result < 0) goto fail; Index: linux-macro-ide/include/scsi/scsi_host.h =================================================================== --- linux-macro-ide.orig/include/scsi/scsi_host.h +++ linux-macro-ide/include/scsi/scsi_host.h @@ -653,6 +653,9 @@ struct Scsi_Host { /* The transport requires the LUN bits NOT to be stored in CDB[1] */ unsigned no_scsi2_lun_in_cdb:1; + /* Allocation length must not exceed actual data length. */ + unsigned no_trailing_allocation_length:1; + /* * Optional work queue to be utilized by the transport */ From patchwork Wed Apr 14 22:39:23 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Maciej W. Rozycki" X-Patchwork-Id: 12203879 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 759BBC433ED for ; Wed, 14 Apr 2021 22:39:33 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 55358611CC for ; Wed, 14 Apr 2021 22:39:33 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234702AbhDNWjx (ORCPT ); Wed, 14 Apr 2021 18:39:53 -0400 Received: from angie.orcam.me.uk ([157.25.102.26]:38956 "EHLO angie.orcam.me.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236609AbhDNWjq (ORCPT ); Wed, 14 Apr 2021 18:39:46 -0400 Received: by angie.orcam.me.uk (Postfix, from userid 500) id DFEFE92009C; Thu, 15 Apr 2021 00:39:23 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by angie.orcam.me.uk (Postfix) with ESMTP id DC4F792009B; Thu, 15 Apr 2021 00:39:23 +0200 (CEST) Date: Thu, 15 Apr 2021 00:39:23 +0200 (CEST) From: "Maciej W. Rozycki" To: Khalid Aziz , "James E.J. Bottomley" , "Martin K. Petersen" cc: Matthew Wilcox , Christoph Hellwig , linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH 4/5] scsi: Avoid using reserved length byte with VPD inquiries In-Reply-To: Message-ID: References: User-Agent: Alpine 2.21 (DEB 202 2017-01-01) MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org As discussed in a previous workaround for a BusLogic BT-958 problem with VPD inquiries with an allocation length of 512 bytes as requested before commit af73623f5f10 ("[SCSI] sd: Reduce buffer size for vpd request") are rejected outright as invalid at least by some SCSI target devices as are any requests with a non-zero value in byte #3: scsi host0: BusLogic BT-958 scsi 0:0:0:0: Direct-Access IBM DDYS-T18350M SA5A PQ: 0 ANSI: 3 scsi 0:0:1:0: Direct-Access SEAGATE ST336607LW 0006 PQ: 0 ANSI: 3 scsi 0:0:5:0: Direct-Access IOMEGA ZIP 100 E.08 PQ: 0 ANSI: 2 [...] scsi0: CCB #36 Target 0: Result 2 Host Adapter Status 00 Target Status 02 scsi0: CDB 12 01 00 01 06 00 scsi0: Sense 70 00 05 00 00 00 00 18 00 00 00 00 24 00 00 C0 00 03 00 [...] sd 0:0:0:0: scsi_vpd_inquiry(0): buf[262] => -5 scsi0: CCB #37 Target 1: Result 2 Host Adapter Status 00 Target Status 02 scsi0: CDB 12 01 00 01 06 00 scsi0: Sense 70 00 05 00 00 00 00 0A 00 00 00 00 24 00 01 C8 00 03 00 [...] sd 0:0:1:0: scsi_vpd_inquiry(0): buf[262] => -5 (here with the buffer size tweaked to 262 so as to verify if a bit in byte #3 of the INQUIRY command is ignored and the length of 6 assumed or tripped over, the `BusLogic=TraceErrors' parameter and trailing sense data zeros trimmed for brevity). Note the sense key of 0x5 denoting an illegal request. For the record with the buffer size of 6 requests for page 0 complete successfully and due to page truncation `scsi_get_vpd_page' proceeds with an attempt to get inexistent page 0x89: sd 0:0:0:0: scsi_vpd_inquiry(0): buf[6] => 7 sd 0:0:1:0: scsi_vpd_inquiry(0): buf[6] => 13 sd 0:0:0:0: scsi_vpd_inquiry(137): buf[6] => -5 sd 0:0:1:0: scsi_vpd_inquiry(137): buf[6] => -5 Upon a further investigation it has turned out at least SCSI-2 considers byte #3 of the INQUIRY command[1] as well as byte #2 of vital product data pages[2] reserved and expects a value of zero there. The response from SCSI-3 devices shown above indicates the same expectation. Therefore it is unsafe to issue INQUIRY requests unconditionally with the allocation length beyond 255, as they may fail with an otherwise supported request or cause undefined behaviour with some hardware. Now we actually never do that as all our callers of `scsi_get_vpd_page' either hardcode the buffer size to a value between 8 and 255 or calculate it from a structure size, of which the largest is: struct c2_inquiry { u8 peripheral_info; /* 0 1 */ u8 page_code; /* 1 1 */ u8 reserved1; /* 2 1 */ u8 page_len; /* 3 1 */ u8 page_id[4]; /* 4 4 */ u8 sw_version[3]; /* 8 3 */ u8 sw_date[3]; /* 11 3 */ u8 features_enabled; /* 14 1 */ u8 max_lun_supported; /* 15 1 */ u8 partitions[239]; /* 16 239 */ /* size: 255, cachelines: 2, members: 10 */ /* last cacheline: 127 bytes */ }; As from commit b3ae8780b429 ("[SCSI] Add EVPD page 0x83 and 0x80 to sysfs") we now also have the SCSI_VPD_PG_LEN macro that reflects the limitation. However for the sake of a possible future requirement to support VPD pages that do have a length exceeding 255 bytes and now that the danger of using the formerly reserved byte #3 of the INQUIRY command has been identified execute calls to `scsi_get_vpd_page' with a request size exceeding 255 bytes in two stages, by determining the actual length of data to be returned first and only then issuing the intended request for full data. References: [1] "Information technology - Small Computer System Interface - 2", WORKING DRAFT, X3T9.2, Project 375D, Revision 10L, 7-SEP-93, Section 8.2.5 "INQUIRY command", pp.104-108 [2] same, Section 8.3.4 "Vital product data parameters", pp.154-159 Signed-off-by: Maciej W. Rozycki Fixes: 881a256d84e6 ("[SCSI] Add VPD helper") --- Hi, NB the SCSI-2 working draft is the only normative reference I have access to, downloaded many years ago and not online anymore. I have more recent vendor documents that do indicate that bytes #3 & #2 respectively are a part of the length field, but based on empirical evidence presented here it is unsafe to unconditionally assume that the bytes can be set to a non-zero value. So I think it will be safest long-term if we handle it correctly right away now that the knowledge is fresh, as past experience with commit af73623f5f10 ("[SCSI] sd: Reduce buffer size for vpd request") indicates the circumstances are not always correctly understood. Maciej --- drivers/scsi/scsi.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) linux-scsi-vpd-inquiry-buffer.diff Index: linux-macro-ide/drivers/scsi/scsi.c =================================================================== --- linux-macro-ide.orig/drivers/scsi/scsi.c +++ linux-macro-ide/drivers/scsi/scsi.c @@ -348,10 +348,15 @@ int scsi_get_vpd_page(struct scsi_device /* * Ask for all the pages supported by this device. Determine the - * actual data length first if so required by the host, e.g. - * BusLogic BT-958. + * actual data length first if the length requested is beyond 255 + * bytes as the high order length byte used to be reserved with + * older SCSI standard revisions and a non-zero value there may + * cause either such an INQUIRY command to be rejected by a target + * or undefined behaviour to occur. Also do so if so required by + * the host, e.g. BusLogic BT-958. */ - if (sdev->host->no_trailing_allocation_length) { + if (buf_len > SCSI_VPD_PG_LEN || + sdev->host->no_trailing_allocation_length) { result = scsi_vpd_inquiry(sdev, buf, 0, min(4, buf_len)); if (result < 4) goto fail; @@ -377,7 +382,8 @@ int scsi_get_vpd_page(struct scsi_device goto fail; found: - if (sdev->host->no_trailing_allocation_length) { + if (buf_len > SCSI_VPD_PG_LEN || + sdev->host->no_trailing_allocation_length) { result = scsi_vpd_inquiry(sdev, buf, page, min(4, buf_len)); if (result < 4) goto fail; From patchwork Wed Apr 14 22:39:28 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Maciej W. Rozycki" X-Patchwork-Id: 12203881 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7BE1CC433ED for ; Wed, 14 Apr 2021 22:39:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 5756B611CC for ; Wed, 14 Apr 2021 22:39:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236520AbhDNWkA (ORCPT ); Wed, 14 Apr 2021 18:40:00 -0400 Received: from angie.orcam.me.uk ([157.25.102.26]:38968 "EHLO angie.orcam.me.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234976AbhDNWjw (ORCPT ); Wed, 14 Apr 2021 18:39:52 -0400 Received: by angie.orcam.me.uk (Postfix, from userid 500) id 059899200B3; Thu, 15 Apr 2021 00:39:29 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by angie.orcam.me.uk (Postfix) with ESMTP id 01F2A92009D; Thu, 15 Apr 2021 00:39:28 +0200 (CEST) Date: Thu, 15 Apr 2021 00:39:28 +0200 (CEST) From: "Maciej W. Rozycki" To: Nix , Khalid Aziz , "James E.J. Bottomley" , "Martin K. Petersen" cc: Bernd Schubert , Christoph Hellwig , linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH 5/5] scsi: Set allocation length to 255 for ATA Information VPD page In-Reply-To: Message-ID: References: User-Agent: Alpine 2.21 (DEB 202 2017-01-01) MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org Set the allocation length to 255 for the ATA Information VPD page requested in the WRITE SAME handler, so as not to limit information examined by `scsi_get_vpd_page' in the supported vital product data pages unnecessarily. Originally it was thought that Areca hardware may have issues with a valid allocation length supplied for a VPD inquiry, however older SCSI standard revisions[1] consider 255 the maximum length allowed and what has later become the high order byte is considered reserved and must be zero with the INQUIRY command. Therefore it was unnecessary to reduce the amount of data requested from 512 as far down as to 64, arbitrarily chosen, and 255 would as well do. With commit b3ae8780b429 ("[SCSI] Add EVPD page 0x83 and 0x80 to sysfs") we have since got the SCSI_VPD_PG_LEN macro, so use that instead. References: [1] "Information technology - Small Computer System Interface - 2", WORKING DRAFT, X3T9.2, Project 375D, Revision 10L, 7-SEP-93, Section 8.2.5 "INQUIRY command", pp.104-108 Signed-off-by: Maciej W. Rozycki Fixes: af73623f5f10 ("[SCSI] sd: Reduce buffer size for vpd request") --- Nix, I can see you're still around. Would you therefore please be so kind as to verify this change with your Areca hardware if you still have it? It looks to me like you were thinking in the right direction with: . Sadly nobody seemed to have paid attention to your observation and neither were different buffer sizes considered (or at least it wasn't mentioned in the discussion). Maciej --- drivers/scsi/sd.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) linux-scsi-write-same-vpd-buffer.diff Index: linux-macro-ide/drivers/scsi/sd.c =================================================================== --- linux-macro-ide.orig/drivers/scsi/sd.c +++ linux-macro-ide/drivers/scsi/sd.c @@ -3076,16 +3076,13 @@ static void sd_read_write_same(struct sc } if (scsi_report_opcode(sdev, buffer, SD_BUF_SIZE, INQUIRY) < 0) { - /* too large values might cause issues with arcmsr */ - int vpd_buf_len = 64; - sdev->no_report_opcodes = 1; /* Disable WRITE SAME if REPORT SUPPORTED OPERATION * CODES is unsupported and the device has an ATA * Information VPD page (SAT). */ - if (!scsi_get_vpd_page(sdev, 0x89, buffer, vpd_buf_len)) + if (!scsi_get_vpd_page(sdev, 0x89, buffer, SCSI_VPD_PG_LEN)) sdev->no_write_same = 1; }