From patchwork Fri May 15 03:41:59 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Calvin Owens X-Patchwork-Id: 6410941 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 720499F1CC for ; Fri, 15 May 2015 03:42:59 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 742B520303 for ; Fri, 15 May 2015 03:42:58 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 61D8E20306 for ; Fri, 15 May 2015 03:42:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934091AbbEODmy (ORCPT ); Thu, 14 May 2015 23:42:54 -0400 Received: from mx0b-00082601.pphosted.com ([67.231.153.30]:19490 "EHLO mx0b-00082601.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933671AbbEODmp (ORCPT ); Thu, 14 May 2015 23:42:45 -0400 Received: from pps.filterd (m0004077 [127.0.0.1]) by mx0b-00082601.pphosted.com (8.14.5/8.14.5) with SMTP id t4F3bSLl011715; Thu, 14 May 2015 20:42: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=EqYqDrXwI9cegpajWeQM9rBTbFZZZudio/iyOG9wKqN/1aCJhg5g7r/IMbKkPAxCSi8C +Ib5DFFIjp9aUMYQsEzay8ecpPcyxvsSVLBJwFUF9A6Dc7K1J1lwELSoUmQLvXHIsv4H gQQusA5fdWKt8iDxFprUxAq9/2hgHeXyK18= Received: from mail.thefacebook.com ([199.201.64.23]) by mx0b-00082601.pphosted.com with ESMTP id 1ud79tr7t7-1 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=NOT); Thu, 14 May 2015 20:42:42 -0700 Received: from devbig030.prn2.facebook.com (192.168.52.13) by PRN-CHUB04.TheFacebook.com (192.168.16.14) with Microsoft SMTP Server id 14.3.195.1; Thu, 14 May 2015 20:42:36 -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: Thu, 14 May 2015 20:41:59 -0700 Message-ID: <1431661322-3097935-4-git-send-email-calvinowens@fb.com> X-Mailer: git-send-email 1.8.1 In-Reply-To: <1431661322-3097935-1-git-send-email-calvinowens@fb.com> References: <20150506184852.GA1007288@mail.thefacebook.com> <1431661322-3097935-1-git-send-email-calvinowens@fb.com> MIME-Version: 1.0 X-Originating-IP: [192.168.52.13] 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-05-15_01:2015-05-14, 2015-05-15, 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=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 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); } }