From patchwork Tue Jun 9 03:50:53 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Calvin Owens X-Patchwork-Id: 6569311 Return-Path: X-Original-To: patchwork-linux-scsi@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 2A6AAC0020 for ; Tue, 9 Jun 2015 03:52:07 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 2DFE320494 for ; Tue, 9 Jun 2015 03:52:06 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 1F5A92046F for ; Tue, 9 Jun 2015 03:52:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932571AbbFIDwB (ORCPT ); Mon, 8 Jun 2015 23:52:01 -0400 Received: from mx0b-00082601.pphosted.com ([67.231.153.30]:55363 "EHLO mx0b-00082601.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932383AbbFIDvo (ORCPT ); Mon, 8 Jun 2015 23:51:44 -0400 Received: from pps.filterd (m0004060 [127.0.0.1]) by mx0b-00082601.pphosted.com (8.14.5/8.14.5) with SMTP id t593pein012946; Mon, 8 Jun 2015 20:51:42 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=fb.com; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-type; s=facebook; bh=3xo6zBSr/ouEXn5wwxJUuTwQ2pPqS69ncyydiZRqDFE=; b=lI09Sn+SXZJ6DTpNknp/95RXewlqvVmynirJoeFXvWU1QPa3JLo3OoxAzX0Tv34tp5Yw mrD7cb91GrdSmlfIT+T1jteTViR74Drf7yVKzmc3rEA/J/RuQDO96OU3gFnciX1Oj0B3 wztYEolLzZ6nPWWQ9pgyY54614a+gtWXOCI= Received: from mail.thefacebook.com ([199.201.64.23]) by mx0b-00082601.pphosted.com with ESMTP id 1uwnharha7-3 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=NOT); Mon, 08 Jun 2015 20:51:42 -0700 Received: from devbig030.prn2.facebook.com (192.168.52.123) by PRN-CHUB08.TheFacebook.com (192.168.16.18) with Microsoft SMTP Server id 14.3.195.1; Mon, 8 Jun 2015 20:51:15 -0700 From: Calvin Owens To: Nagalakshmi Nandigama , Praveen Krishnamoorthy , Sreekanth Reddy , Abhijit Mahajan CC: , , , , Subject: [PATCH 3/6] Fix unsafe sas_device_list usage Date: Mon, 8 Jun 2015 20:50:53 -0700 Message-ID: <1433821856-2815280-4-git-send-email-calvinowens@fb.com> X-Mailer: git-send-email 1.8.1 In-Reply-To: <1433821856-2815280-1-git-send-email-calvinowens@fb.com> References: <1431661322-3097935-1-git-send-email-calvinowens@fb.com> <1433821856-2815280-1-git-send-email-calvinowens@fb.com> MIME-Version: 1.0 X-Originating-IP: [192.168.52.123] X-Proofpoint-Spam-Reason: safe X-FB-Internal: Safe X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:5.14.151, 1.0.33, 0.0.0000 definitions=2015-06-09_04:2015-06-08, 2015-06-09, 1970-01-01 signatures=0 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 We cannot iterate over the list without holding a lock for the entire duration, or we risk corrupting random memory if items are added or deleted as we iterate. This refactors code such that it always holds the lock when iterating on or accessing the sas_device_list. Signed-off-by: Calvin Owens --- drivers/scsi/mpt2sas/mpt2sas_scsih.c | 83 +++++++++++++++++++++++++++--------- 1 file changed, 62 insertions(+), 21 deletions(-) diff --git a/drivers/scsi/mpt2sas/mpt2sas_scsih.c b/drivers/scsi/mpt2sas/mpt2sas_scsih.c index ad6ceb7e..9645055 100644 --- a/drivers/scsi/mpt2sas/mpt2sas_scsih.c +++ b/drivers/scsi/mpt2sas/mpt2sas_scsih.c @@ -7104,6 +7104,7 @@ _scsih_remove_unresponding_sas_devices(struct MPT2SAS_ADAPTER *ioc) struct _raid_device *raid_device, *raid_device_next; struct list_head tmp_list; unsigned long flags; + LIST_HEAD(head); printk(MPT2SAS_INFO_FMT "removing unresponding devices: start\n", ioc->name); @@ -7111,14 +7112,29 @@ _scsih_remove_unresponding_sas_devices(struct MPT2SAS_ADAPTER *ioc) /* removing unresponding end devices */ printk(MPT2SAS_INFO_FMT "removing unresponding devices: end-devices\n", ioc->name); + + /* + * Iterate, pulling off devices marked as non-responding. We become the + * owner for the reference the list had on any object we prune. + */ + spin_lock_irqsave(&ioc->sas_device_lock, flags); list_for_each_entry_safe(sas_device, sas_device_next, - &ioc->sas_device_list, list) { + &ioc->sas_device_list, list) { if (!sas_device->responding) - mpt2sas_device_remove_by_sas_address(ioc, - sas_device->sas_address); + list_move_tail(&sas_device->list, &head); else sas_device->responding = 0; } + spin_unlock_irqrestore(&ioc->sas_device_lock, flags); + + /* + * Now, uninitialize and remove the unresponding devices we pruned. + */ + list_for_each_entry_safe(sas_device, sas_device_next, &head, list) { + _scsih_remove_device(ioc, sas_device); + list_del_init(&sas_device->list); + sas_device_put(sas_device); + } /* removing unresponding volumes */ if (ioc->ir_firmware) { @@ -8055,6 +8071,37 @@ _scsih_probe_raid(struct MPT2SAS_ADAPTER *ioc) } } +static struct _sas_device *dequeue_next_sas_device(struct MPT2SAS_ADAPTER *ioc) +{ + struct _sas_device *sas_device = NULL; + unsigned long flags; + + spin_lock_irqsave(&ioc->sas_device_lock, flags); + if (!list_empty(&ioc->sas_device_init_list)) { + sas_device = list_first_entry(&ioc->sas_device_init_list, + struct _sas_device, list); + list_del_init(&sas_device->list); + } + spin_unlock_irqrestore(&ioc->sas_device_lock, flags); + + /* + * If an item was dequeued, the caller now owns the reference that was + * previously owned by the list + */ + return sas_device; +} + +static void sas_device_make_active(struct MPT2SAS_ADAPTER *ioc, + struct _sas_device *sas_device) +{ + unsigned long flags; + + spin_lock_irqsave(&ioc->sas_device_lock, flags); + sas_device_get(sas_device); + list_add_tail(&sas_device->list, &ioc->sas_device_list); + spin_unlock_irqrestore(&ioc->sas_device_lock, flags); +} + /** * _scsih_probe_sas - reporting sas devices to sas transport * @ioc: per adapter object @@ -8064,34 +8111,28 @@ _scsih_probe_raid(struct MPT2SAS_ADAPTER *ioc) static void _scsih_probe_sas(struct MPT2SAS_ADAPTER *ioc) { - struct _sas_device *sas_device, *next; - unsigned long flags; - - /* SAS Device List */ - list_for_each_entry_safe(sas_device, next, &ioc->sas_device_init_list, - list) { + struct _sas_device *sas_device; - if (ioc->hide_drives) - continue; + if (ioc->hide_drives) + return; + while ((sas_device = dequeue_next_sas_device(ioc))) { if (!mpt2sas_transport_port_add(ioc, sas_device->handle, - sas_device->sas_address_parent)) { - list_del(&sas_device->list); - kfree(sas_device); + sas_device->sas_address_parent)) { + sas_device_put(sas_device); continue; } else if (!sas_device->starget) { if (!ioc->is_driver_loading) { mpt2sas_transport_port_remove(ioc, - sas_device->sas_address, - sas_device->sas_address_parent); - list_del(&sas_device->list); - kfree(sas_device); + sas_device->sas_address, + sas_device->sas_address_parent); + sas_device_put(sas_device); continue; } } - spin_lock_irqsave(&ioc->sas_device_lock, flags); - list_move_tail(&sas_device->list, &ioc->sas_device_list); - spin_unlock_irqrestore(&ioc->sas_device_lock, flags); + + sas_device_make_active(ioc, sas_device); + sas_device_put(sas_device); } }