From mboxrd@z Thu Jan 1 00:00:00 1970 From: James Bottomley Subject: Re: [PATCH v2 17/19] qla2xxx: prevent bounds-check bypass via speculative execution Date: Thu, 11 Jan 2018 17:19:16 -0800 Message-ID: <1515719956.3056.37.camel@linux.vnet.ibm.com> References: <151571798296.27429.7166552848688034184.stgit@dwillia2-desk3.amr.corp.intel.com> <151571807774.27429.7382333750792304971.stgit@dwillia2-desk3.amr.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit Return-path: In-Reply-To: <151571807774.27429.7382333750792304971.stgit@dwillia2-desk3.amr.corp.intel.com> Sender: linux-scsi-owner@vger.kernel.org To: Dan Williams , linux-kernel@vger.kernel.org Cc: linux-arch@vger.kernel.org, "Martin K. Petersen" , linux-scsi@vger.kernel.org, kernel-hardening@lists.openwall.com, qla2xxx-upstream@qlogic.com, tglx@linutronix.de, torvalds@linux-foundation.org, akpm@linux-foundation.org, Elena Reshetova , alan@linux.intel.com List-Id: linux-arch.vger.kernel.org On Thu, 2018-01-11 at 16:47 -0800, Dan Williams wrote: > Static analysis reports that 'handle' may be a user controlled value > that is used as a data dependency to read 'sp' from the > 'req->outstanding_cmds' array. Greg already told you it comes from hardware, specifically the hardware response queue.  If you don't believe him, I can confirm it's quite definitely all copied from the iomem where the mailbox response is, so it can't be a user controlled value (well, unless the user has some influence over the firmware of the qla2xxx  controller, which probably means you have other things to worry about than speculative information leaks). I think what it actually is is a handle returned in the mailbox that's used to find other mailbox entries in different queues (the packet handle actually contains an entry in the lower 16 bits and a queue designation in the upper).  Perhaps the qla2xxx people should comment on this because the code seems to check and print an error if there's a problem with the handle being too big, but we don't check the que value and use it blindly to read into the req_q_map: if handle could be wrong, couldn't que? James From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:39084 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S932422AbeALBVr (ORCPT ); Thu, 11 Jan 2018 20:21:47 -0500 Received: from pps.filterd (m0098413.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.22/8.16.0.22) with SMTP id w0C1Ljg8014046 for ; Thu, 11 Jan 2018 20:21:46 -0500 Received: from e16.ny.us.ibm.com (e16.ny.us.ibm.com [129.33.205.206]) by mx0b-001b2d01.pphosted.com with ESMTP id 2feh5rw72e-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Thu, 11 Jan 2018 20:21:45 -0500 Received: from localhost by e16.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 11 Jan 2018 20:19:24 -0500 Subject: Re: [PATCH v2 17/19] qla2xxx: prevent bounds-check bypass via speculative execution From: James Bottomley Date: Thu, 11 Jan 2018 17:19:16 -0800 In-Reply-To: <151571807774.27429.7382333750792304971.stgit@dwillia2-desk3.amr.corp.intel.com> References: <151571798296.27429.7166552848688034184.stgit@dwillia2-desk3.amr.corp.intel.com> <151571807774.27429.7382333750792304971.stgit@dwillia2-desk3.amr.corp.intel.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Message-ID: <1515719956.3056.37.camel@linux.vnet.ibm.com> Sender: linux-arch-owner@vger.kernel.org List-ID: To: Dan Williams , linux-kernel@vger.kernel.org Cc: linux-arch@vger.kernel.org, "Martin K. Petersen" , linux-scsi@vger.kernel.org, kernel-hardening@lists.openwall.com, qla2xxx-upstream@qlogic.com, tglx@linutronix.de, torvalds@linux-foundation.org, akpm@linux-foundation.org, Elena Reshetova , alan@linux.intel.com Message-ID: <20180112011916.DlH76mT6ZUH2h_SaOv1MKqFsDJxQOzdOjJrIjo5qVQI@z> On Thu, 2018-01-11 at 16:47 -0800, Dan Williams wrote: > Static analysis reports that 'handle' may be a user controlled value > that is used as a data dependency to read 'sp' from the > 'req->outstanding_cmds' array. Greg already told you it comes from hardware, specifically the hardware response queue.  If you don't believe him, I can confirm it's quite definitely all copied from the iomem where the mailbox response is, so it can't be a user controlled value (well, unless the user has some influence over the firmware of the qla2xxx  controller, which probably means you have other things to worry about than speculative information leaks). I think what it actually is is a handle returned in the mailbox that's used to find other mailbox entries in different queues (the packet handle actually contains an entry in the lower 16 bits and a queue designation in the upper).  Perhaps the qla2xxx people should comment on this because the code seems to check and print an error if there's a problem with the handle being too big, but we don't check the que value and use it blindly to read into the req_q_map: if handle could be wrong, couldn't que? James