From patchwork Thu May 11 17:21:16 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Nicholas A. Bellinger" X-Patchwork-Id: 9722597 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 6747C601E7 for ; Thu, 11 May 2017 17:21:45 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 5889D283B9 for ; Thu, 11 May 2017 17:21:45 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 4D32B286D2; Thu, 11 May 2017 17:21:45 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 5A0D5283B9 for ; Thu, 11 May 2017 17:21:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932660AbdEKRVm (ORCPT ); Thu, 11 May 2017 13:21:42 -0400 Received: from mail.linux-iscsi.org ([67.23.28.174]:51521 "EHLO linux-iscsi.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757817AbdEKRVa (ORCPT ); Thu, 11 May 2017 13:21:30 -0400 Received: from [172.16.2.183] (unknown [50.225.59.10]) (using SSLv3 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) (Authenticated sender: nab) by linux-iscsi.org (Postfix) with ESMTPSA id 7538640B0D; Thu, 11 May 2017 17:23:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=linux-iscsi.org; s=default.private; t=1494523402; bh=OmbHAAivuAZ0xSoObBuhMkO9KIgQqrO T+41HuEXCpk4=; h=Message-ID:Subject:From:To:Cc:Date:In-Reply-To: References:Content-Type:Mime-Version:Content-Transfer-Encoding; b=Toim49NsOHlnySXmxaKajPpO+ITNOFPt1tDzhp+yuYJKTfcGYoMuLsJTCd5zXyZ9O D0qe1PpT/ZHz89vZLwDPEekxrYekTJX8PYUoi0dv5vdry4Z+b3OP5ZL9D0ZwwgdkGlQ 0Ryz47dgK6D+KNPXTWX1bREsyaFzy2Sfb9lhmnU= Message-ID: <1494523276.14920.2.camel@haakon3.risingtidesystems.com> Subject: Re: [PATCH v2 0/6] SCSI target patches for kernel v4.12 From: "Nicholas A. Bellinger" To: Bart Van Assche Cc: "target-devel@vger.kernel.org" Date: Thu, 11 May 2017 10:21:16 -0700 In-Reply-To: <1494515911.2489.2.camel@sandisk.com> References: <20170510203736.12462-1-bart.vanassche@sandisk.com> <1494481655.16894.148.camel@haakon3.risingtidesystems.com> <1494485207.16894.166.camel@haakon3.risingtidesystems.com> <1494515911.2489.2.camel@sandisk.com> X-Mailer: Evolution 3.4.4-1 Mime-Version: 1.0 Sender: target-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: target-devel@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Thu, 2017-05-11 at 15:18 +0000, Bart Van Assche wrote: > On Wed, 2017-05-10 at 23:46 -0700, Nicholas A. Bellinger wrote: > > then I'll go ahead and push the revert here: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/nab/target-pending.git/commit/?h=for-next&id=69be17042db730092d25691dd85868a41d4d99ac > > Hello Nic, > > As you know any patch that is sent to Linus should be posted on the > appropriate mailing list first. Posting an URL to a patch is not sufficient. > It's a revert with more than appropriate context that has been discussed ad-nauseum for the last number of days with no actual end-user impact. But if you insist, here you go. This reverts commit 0e2eb7d12eaa8e391bf5615d4271bb87a649caaa Author: Bart Van Assche Date: Thu Mar 30 10:12:39 2017 -0700 target: Fix VERIFY and WRITE VERIFY command parsing This patch broke existing behaviour for WRITE_VERIFY because it dropped the original SCF_SCSI_DATA_CDB assignment for bytchk = 0 so target_cmd_size_check() no longer rejected this case, allowing an overflow case to trigger an OOPs in iscsi-target. Since the short term and long term fixes are still being discussed, revert it for now since it's late in the merge window and try again in v4.13-rc1. Conflicts: drivers/target/target_core_sbc.c Signed-off-by: Nicholas Bellinger --- drivers/target/target_core_sbc.c | 74 ++++++---------------------------------- 1 file changed, 10 insertions(+), 64 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c index a0ad618..4316f7b 100644 --- a/drivers/target/target_core_sbc.c +++ b/drivers/target/target_core_sbc.c @@ -831,60 +831,6 @@ static sense_reason_t compare_and_write_callback(struct se_cmd *cmd, bool succes return 0; } -/** - * sbc_parse_verify - parse VERIFY, VERIFY_16 and WRITE VERIFY commands - * @cmd: (in) structure that describes the SCSI command to be parsed. - * @sectors: (out) Number of logical blocks on the storage medium that will be - * affected by the SCSI command. - * @bufflen: (out) Expected length of the SCSI Data-Out buffer. - */ -static sense_reason_t sbc_parse_verify(struct se_cmd *cmd, int *sectors, - u32 *bufflen) -{ - struct se_device *dev = cmd->se_dev; - u8 *cdb = cmd->t_task_cdb; - u8 bytchk = (cdb[1] >> 1) & 3; - sense_reason_t ret; - - switch (cdb[0]) { - case VERIFY: - case WRITE_VERIFY: - *sectors = transport_get_sectors_10(cdb); - cmd->t_task_lba = transport_lba_32(cdb); - break; - case VERIFY_16: - case WRITE_VERIFY_16: - *sectors = transport_get_sectors_16(cdb); - cmd->t_task_lba = transport_lba_64(cdb); - break; - default: - WARN_ON_ONCE(true); - return TCM_UNSUPPORTED_SCSI_OPCODE; - } - - if (sbc_check_dpofua(dev, cmd, cdb)) - return TCM_INVALID_CDB_FIELD; - - ret = sbc_check_prot(dev, cmd, cdb, *sectors, true); - if (ret) - return ret; - - switch (bytchk) { - case 0: - *bufflen = 0; - break; - case 1: - *bufflen = sbc_get_size(cmd, *sectors); - cmd->se_cmd_flags |= SCF_SCSI_DATA_CDB; - break; - default: - pr_err("Unsupported BYTCHK value %d for SCSI opcode %#x\n", - bytchk, cdb[0]); - return TCM_INVALID_CDB_FIELD; - } - return TCM_NO_SENSE; -} - sense_reason_t sbc_parse_cdb(struct se_cmd *cmd, struct sbc_ops *ops) { @@ -952,6 +898,7 @@ static sense_reason_t sbc_parse_verify(struct se_cmd *cmd, int *sectors, cmd->execute_cmd = sbc_execute_rw; break; case WRITE_10: + case WRITE_VERIFY: sectors = transport_get_sectors_10(cdb); cmd->t_task_lba = transport_lba_32(cdb); @@ -965,13 +912,6 @@ static sense_reason_t sbc_parse_verify(struct se_cmd *cmd, int *sectors, cmd->se_cmd_flags |= SCF_SCSI_DATA_CDB; cmd->execute_cmd = sbc_execute_rw; break; - case WRITE_VERIFY: - case WRITE_VERIFY_16: - ret = sbc_parse_verify(cmd, §ors, &size); - if (ret) - return ret; - cmd->execute_cmd = sbc_execute_rw; - goto check_lba; case WRITE_12: sectors = transport_get_sectors_12(cdb); cmd->t_task_lba = transport_lba_32(cdb); @@ -987,6 +927,7 @@ static sense_reason_t sbc_parse_verify(struct se_cmd *cmd, int *sectors, cmd->execute_cmd = sbc_execute_rw; break; case WRITE_16: + case WRITE_VERIFY_16: sectors = transport_get_sectors_16(cdb); cmd->t_task_lba = transport_lba_64(cdb); @@ -1169,9 +1110,14 @@ static sense_reason_t sbc_parse_verify(struct se_cmd *cmd, int *sectors, break; case VERIFY: case VERIFY_16: - ret = sbc_parse_verify(cmd, §ors, &size); - if (ret) - return ret; + size = 0; + if (cdb[0] == VERIFY) { + sectors = transport_get_sectors_10(cdb); + cmd->t_task_lba = transport_lba_32(cdb); + } else { + sectors = transport_get_sectors_16(cdb); + cmd->t_task_lba = transport_lba_64(cdb); + } cmd->execute_cmd = sbc_emulate_noop; goto check_lba; case REZERO_UNIT: