From patchwork Wed Sep 9 06:21:27 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Nicholas A. Bellinger" X-Patchwork-Id: 7144591 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 51C769F314 for ; Wed, 9 Sep 2015 06:24:16 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 39F9D20977 for ; Wed, 9 Sep 2015 06:24:15 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id BB2EB20972 for ; Wed, 9 Sep 2015 06:24:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751441AbbIIGXj (ORCPT ); Wed, 9 Sep 2015 02:23:39 -0400 Received: from mail.linux-iscsi.org ([67.23.28.174]:34772 "EHLO linux-iscsi.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750744AbbIIGXi (ORCPT ); Wed, 9 Sep 2015 02:23:38 -0400 Received: from [192.168.1.68] (75-37-194-224.lightspeed.lsatca.sbcglobal.net [75.37.194.224]) (using SSLv3 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) (Authenticated sender: nab) by linux-iscsi.org (Postfix) with ESMTPSA id BBC6422DA38; Wed, 9 Sep 2015 06:17:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=linux-iscsi.org; s=default.private; t=1441779452; bh=qRxjjyRJiVC1WqI3e1rtbWHLezlLrGj p06wNQ2Gc37M=; h=Message-ID:Subject:From:To:Cc:In-Reply-To: References:Content-Type:Date:Mime-Version: Content-Transfer-Encoding; b=dI8VoDbmXyThvW6aT5f7t/L9QdM5ZCMcnDrWr unGdnfv69pjGoQG/GJApz8acpbVqufv+HWIxI71hr8A2nWdyHQkLv6h2l9OBa2XHkvY ki/3o2w9M/U3drBxEXXKRiR96L0gI4hgXYxkZiE6xNhBFBaQCql7yQm7SK3aw1zeH/M = Message-ID: <1441779687.26610.6.camel@haakon3.risingtidesystems.com> Subject: Re: [PATCH-v2 1/2] mpt3sas: Refcount sas_device objects and fix unsafe list usage From: "Nicholas A. Bellinger" To: Sreekanth Reddy Cc: "Nicholas A. Bellinger" , linux-scsi , linux-kernel , James Bottomley , Calvin Owens , Christoph Hellwig , "MPT-FusionLinux.pdl" , kernel-team , Chaitra Basappa , "Martin K. Petersen" In-Reply-To: References: <1440921279-9973-1-git-send-email-nab@daterainc.com> <1440921279-9973-2-git-send-email-nab@daterainc.com> Date: Tue, 08 Sep 2015 23:21:27 -0700 Mime-Version: 1.0 X-Mailer: Evolution 3.4.4-1 Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID,T_RP_MATCHES_RCVD,UNPARSEABLE_RELAY autolearn=ham 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 Hi Sreekanth, On Tue, 2015-09-08 at 17:25 +0530, Sreekanth Reddy wrote: > On Sun, Aug 30, 2015 at 1:24 PM, Nicholas A. Bellinger > wrote: > > From: Nicholas Bellinger > > > > These objects can be referenced concurrently throughout the driver, we > > need a way to make sure threads can't delete them out from under > > each other. This patch adds the refcount, and refactors the code to use > > it. > > > > Additionally, we cannot iterate over the sas_device_list without > > holding the lock, or we risk corrupting random memory if items are > > added or deleted as we iterate. This patch refactors _scsih_probe_sas() > > to use the sas_device_list in a safe way. > > > > This patch is a port of Calvin's PATCH-v4 for mpt2sas code, atop > > mpt3sas changes in scsi.git/for-next. > > > > Cc: Calvin Owens > > Cc: Christoph Hellwig > > Cc: Sreekanth Reddy > > Cc: MPT-FusionLinux.pdl > > Signed-off-by: Nicholas Bellinger > > --- > > drivers/scsi/mpt3sas/mpt3sas_base.h | 25 +- > > drivers/scsi/mpt3sas/mpt3sas_scsih.c | 479 +++++++++++++++++++++---------- > > drivers/scsi/mpt3sas/mpt3sas_transport.c | 18 +- > > 3 files changed, 364 insertions(+), 158 deletions(-) > > diff --git a/drivers/scsi/mpt3sas/mpt3sas_transport.c b/drivers/scsi/mpt3sas/mpt3sas_transport.c > > index 70fd019..6074b11 100644 > > --- a/drivers/scsi/mpt3sas/mpt3sas_transport.c > > +++ b/drivers/scsi/mpt3sas/mpt3sas_transport.c > > @@ -734,7 +734,7 @@ mpt3sas_transport_port_add(struct MPT3SAS_ADAPTER *ioc, u16 handle, > > rphy->identify = mpt3sas_port->remote_identify; > > > > if (mpt3sas_port->remote_identify.device_type == SAS_END_DEVICE) { > > [Sreekanth] Here also sas_device_lock spin lock needs to be acquired > before calling > __mpt3sas_get_sdev_by_addr() function. > Thanks for your review. Fixed with the following incremental patch atop for-next-merge code. Please review. Thank you, From 4004584675d24d8e886b03c013074cdfb76a0864 Mon Sep 17 00:00:00 2001 From: Nicholas Bellinger Date: Tue, 8 Sep 2015 23:05:49 -0700 Subject: [PATCH] mpt3sas: Add missing ioc->sas_device_lock in mpt3sas_transport_port_add This patch adds the missing struct MPT3SAS_ADAPTER->sas_device_lock usage in __mpt3sas_get_sdev_by_addr() lookup to avoid a NULL pointer dereference when &ioc->sas_device_list or &ioc->sas_device_init_list changes from below without a proper sas_device_get(sas_device) reference held. Reported-by: Sreekanth Reddy Cc: Calvin Owens Cc: Christoph Hellwig Cc: Martin K. Petersen Signed-off-by: Nicholas Bellinger --- drivers/scsi/mpt3sas/mpt3sas_transport.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/scsi/mpt3sas/mpt3sas_transport.c b/drivers/scsi/mpt3sas/mpt3sas_transport.c index 6074b11..cc55907 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_transport.c +++ b/drivers/scsi/mpt3sas/mpt3sas_transport.c @@ -734,8 +734,10 @@ mpt3sas_transport_port_add(struct MPT3SAS_ADAPTER *ioc, u16 handle, rphy->identify = mpt3sas_port->remote_identify; if (mpt3sas_port->remote_identify.device_type == SAS_END_DEVICE) { + spin_lock_irqsave(&ioc->sas_device_lock, flags); sas_device = __mpt3sas_get_sdev_by_addr(ioc, mpt3sas_port->remote_identify.sas_address); + spin_unlock_irqrestore(&ioc->sas_device_lock, flags); if (!sas_device) { dfailprintk(ioc, printk(MPT3SAS_FMT "failure at %s:%d/%s()!\n",