From patchwork Thu Jun 25 16:00:14 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Brian King X-Patchwork-Id: 6676331 Return-Path: X-Original-To: patchwork-linux-scsi@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 884359F380 for ; Thu, 25 Jun 2015 16:00:36 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 09B5E206EC for ; Thu, 25 Jun 2015 16:00:31 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 2AA5920553 for ; Thu, 25 Jun 2015 16:00:29 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752515AbbFYQA1 (ORCPT ); Thu, 25 Jun 2015 12:00:27 -0400 Received: from e19.ny.us.ibm.com ([129.33.205.209]:58514 "EHLO e19.ny.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752521AbbFYQA0 (ORCPT ); Thu, 25 Jun 2015 12:00:26 -0400 Received: from /spool/local by e19.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 25 Jun 2015 12:00:25 -0400 Received: from d01dlp03.pok.ibm.com (9.56.250.168) by e19.ny.us.ibm.com (146.89.104.206) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Thu, 25 Jun 2015 12:00:22 -0400 X-Helo: d01dlp03.pok.ibm.com X-MailFrom: brking@linux.vnet.ibm.com X-RcptTo: stable@vger.kernel.org Received: from b01cxnp22036.gho.pok.ibm.com (b01cxnp22036.gho.pok.ibm.com [9.57.198.26]) by d01dlp03.pok.ibm.com (Postfix) with ESMTP id 53E05C90046; Thu, 25 Jun 2015 11:51:28 -0400 (EDT) Received: from d01av01.pok.ibm.com (d01av01.pok.ibm.com [9.56.224.215]) by b01cxnp22036.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t5PG0M5e44892252; Thu, 25 Jun 2015 16:00:22 GMT Received: from d01av01.pok.ibm.com (localhost [127.0.0.1]) by d01av01.pok.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t5PG0KvH005087; Thu, 25 Jun 2015 12:00:21 -0400 Received: from skaro.rchland.ibm.com (sig-9-65-168-72.ibm.com [9.65.168.72]) by d01av01.pok.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id t5PG0G13004498; Thu, 25 Jun 2015 12:00:17 -0400 Message-ID: <558C258E.4000002@linux.vnet.ibm.com> Date: Thu, 25 Jun 2015 11:00:14 -0500 From: Brian King User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Jiri Slaby , James.Bottomley@HansenPartnership.com CC: linux-scsi@vger.kernel.org, wenxiong@linux.vnet.ibm.com, krisman@linux.vnet.ibm.com, stable@vger.kernel.org Subject: [PATCHv2 1/1] ipr: Fix invalid array indexing for HRRQ References: <201506251344.t5PDiZLv025988@d03av03.boulder.ibm.com> <558C11F0.6020505@suse.cz> <558C23F4.8050709@linux.vnet.ibm.com> In-Reply-To: <558C23F4.8050709@linux.vnet.ibm.com> X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 15062516-0057-0000-0000-0000008D5752 Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org X-Spam-Status: No, score=-8.3 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Updated version per comments from Jiri Slaby. Thanks! 8< Fixes another signed / unsigned array indexing bug in the ipr driver. Currently, when hrrq_index wraps, it becomes a negative number. We do the modulo, but still have a negative number, so we end up indexing backwards in the array. Given where the hrrq array is located in memory, we probably won't actually reference memory we don't own, but nonetheless ipr is still looking at data within struct ipr_ioa_cfg and interpreting it as struct ipr_hrr_queue data, so bad things could certainly happen. Each ipr adapter has anywhere from 1 to 16 HRRQs. By default, we use 2 on new adapters. Let's take an example: Assume ioa_cfg->hrrq_index=0x7fffffffe and ioa_cfg->hrrq_num=4: The atomic_add_return will then return -1. We mod this with 3 and get -2, add one and get -1 for an array index. On adapters which support more than a single HRRQ, we dedicate HRRQ to adapter initialization and error interrupts so that we can optimize the other queues for fast path I/O. So all normal I/O uses HRRQ 1-15. So we want to spread the I/O requests across those HRRQs. With the default module parameter settings, this bug won't hit, only when someone sets the ipr.number_of_msix parameter to a value larger than 3 is when bad things start to happen. Signed-off-by: Brian King Tested-by: Wen Xiong Cc: Reviewed-by: Wen Xiong Reviewed-by: Gabriel Krisman Bertazi --- drivers/scsi/ipr.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff -puN drivers/scsi/ipr.c~ipr_hrrq_index_fix drivers/scsi/ipr.c --- linux/drivers/scsi/ipr.c~ipr_hrrq_index_fix 2015-06-23 11:43:18.151741523 -0500 +++ linux-bjking1/drivers/scsi/ipr.c 2015-06-25 10:54:24.954615461 -0500 @@ -1052,10 +1052,15 @@ static void ipr_send_blocking_cmd(struct static int ipr_get_hrrq_index(struct ipr_ioa_cfg *ioa_cfg) { + unsigned int hrrq; + if (ioa_cfg->hrrq_num == 1) - return 0; - else - return (atomic_add_return(1, &ioa_cfg->hrrq_index) % (ioa_cfg->hrrq_num - 1)) + 1; + hrrq = 0; + else { + hrrq = atomic_add_return(1, &ioa_cfg->hrrq_index); + hrrq = (hrrq % (ioa_cfg->hrrq_num - 1)) + 1; + } + return hrrq; } /**