From patchwork Sun May 7 22:55:59 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: 9715611 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 DED0E60365 for ; Sun, 7 May 2017 22:56:03 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D9375223C7 for ; Sun, 7 May 2017 22:56:03 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id CD29A26224; Sun, 7 May 2017 22:56:03 +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 431B7223C7 for ; Sun, 7 May 2017 22:56:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752238AbdEGW4B (ORCPT ); Sun, 7 May 2017 18:56:01 -0400 Received: from mail.linux-iscsi.org ([67.23.28.174]:54541 "EHLO linux-iscsi.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751774AbdEGW4B (ORCPT ); Sun, 7 May 2017 18:56:01 -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 CCAC640B0D; Sun, 7 May 2017 22:57:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=linux-iscsi.org; s=default.private; t=1494197868; bh=4S/0ipCxz2324eo19ImxtCrR3M/ZFCS otoNNuGH+8F4=; h=Message-ID:Subject:From:To:Cc:Date:In-Reply-To: References:Content-Type:Mime-Version:Content-Transfer-Encoding; b=lqm++v0+AqgREFIuPqSL9/6j27s4KxJT+XtrI0FvX6bHhFuQvbm2dQaa5wm6GWS16 DBfmpZhgDIzlJ1A1A2J3zfiLQtmicihkVYeKGS2Rt/OYzATgdT257nYxm9w/dyqGMaR RTy6m490H0AG/h3e4EODQvY3dVgKDEWtxZqTGMM= Message-ID: <1494197759.30469.39.camel@haakon3.risingtidesystems.com> Subject: Re: [PATCH 18/19] target/iscsi: Avoid that CDB parser bugs trigger a kernel crash From: "Nicholas A. Bellinger" To: Bart Van Assche Cc: target-devel@vger.kernel.org, Hannes Reinecke , Christoph Hellwig , Andy Grover , David Disseldorp Date: Sun, 07 May 2017 15:55:59 -0700 In-Reply-To: <20170504225102.8931-19-bart.vanassche@sandisk.com> References: <20170504225102.8931-1-bart.vanassche@sandisk.com> <20170504225102.8931-19-bart.vanassche@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-04 at 15:51 -0700, Bart Van Assche wrote: > If the code for parsing a CDB sets se_cmd.data_length to a value > that is lower than the size of the SCSI Data-Out buffer then a > buffer overflow occurs in the iSCSI target driver while receiving > the Data-Out buffer. Make the code for receiving that buffer more > robust by checking the bounds of the allocated iovec. This patch > fixes the following crash: > > BUG: unable to handle kernel NULL pointer dereference at 00000000 > 00000014 > RIP: 0010:iscsit_map_iovec+0x120/0x190 [iscsi_target_mod] > Call Trace: > iscsit_get_rx_pdu+0x8a2/0xe00 [iscsi_target_mod] > iscsi_target_rx_thread+0x6e/0xa0 [iscsi_target_mod] > kthread+0x109/0x140 > Again, this OOPs is a consequence of your earlier sbc_parse_verify() patch not setting SCF_SCSI_DATA_CDB and allowing unsupported WRITE overflow to be processed. Here is what I'm applying to for-next to address these regressions. First, is to always set SCF_SCSI_DATA_CDB when we expect to transfer data regardless of bytchk, and don't incorrect set the 'bufflen = 0' for bytchk = 0 because it will still be transferring data. --- 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..2cc8753 100644 --- a/drivers/target/target_core_sbc.c +++ b/drivers/target/target_core_sbc.c @@ -836,10 +836,8 @@ static sense_reason_t compare_and_write_callback(struct se_cmd *cmd, bool succes * @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) +static sense_reason_t sbc_parse_verify(struct se_cmd *cmd, unsigned int *sectors) { struct se_device *dev = cmd->se_dev; u8 *cdb = cmd->t_task_cdb; @@ -871,10 +869,7 @@ static sense_reason_t sbc_parse_verify(struct se_cmd *cmd, int *sectors, 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: @@ -967,7 +962,7 @@ static sense_reason_t sbc_parse_verify(struct se_cmd *cmd, int *sectors, break; case WRITE_VERIFY: case WRITE_VERIFY_16: - ret = sbc_parse_verify(cmd, §ors, &size); + ret = sbc_parse_verify(cmd, §ors); if (ret) return ret; cmd->execute_cmd = sbc_execute_rw; @@ -1169,7 +1164,7 @@ 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); + ret = sbc_parse_verify(cmd, §ors); if (ret) return ret; cmd->execute_cmd = sbc_emulate_noop;