From patchwork Thu Oct 1 04:38:45 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Junichi Nomura X-Patchwork-Id: 7303991 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 3F5839F39B for ; Thu, 1 Oct 2015 04:42:53 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 4C0FD20780 for ; Thu, 1 Oct 2015 04:42:52 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 4866C2077F for ; Thu, 1 Oct 2015 04:42:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751780AbbJAEmu (ORCPT ); Thu, 1 Oct 2015 00:42:50 -0400 Received: from TYO201.gate.nec.co.jp ([210.143.35.51]:51849 "EHLO tyo201.gate.nec.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750920AbbJAEmt convert rfc822-to-8bit (ORCPT ); Thu, 1 Oct 2015 00:42:49 -0400 Received: from mailgate3.nec.co.jp ([10.7.69.160]) by tyo201.gate.nec.co.jp (8.13.8/8.13.4) with ESMTP id t914gfKa015636; Thu, 1 Oct 2015 13:42:41 +0900 (JST) Received: from mailsv4.nec.co.jp (imss61.nec.co.jp [10.7.69.156]) by mailgate3.nec.co.jp (8.11.7/3.7W-MAILGATE-NEC) with ESMTP id t914geN18807; Thu, 1 Oct 2015 13:42:40 +0900 (JST) Received: from mail02.kamome.nec.co.jp (mail02.kamome.nec.co.jp [10.25.43.5]) by mailsv4.nec.co.jp (8.13.8/8.13.4) with ESMTP id t914gdPH023836; Thu, 1 Oct 2015 13:42:40 +0900 (JST) Received: from bpxc99gp.gisp.nec.co.jp ([10.38.151.139] [10.38.151.139]) by mail03.kamome.nec.co.jp with ESMTP id BT-MMP-2340542; Thu, 1 Oct 2015 13:38:46 +0900 Received: from BPXM12GP.gisp.nec.co.jp ([169.254.2.181]) by BPXC11GP.gisp.nec.co.jp ([10.38.151.139]) with mapi id 14.03.0224.002; Thu, 1 Oct 2015 13:38:45 +0900 From: Junichi Nomura To: Christoph Hellwig CC: linux-scsi , Hannes Reinecke Subject: Re: [REGRESSION v4.3] scsi_dh: use-after-free when removing scsi device Thread-Topic: [REGRESSION v4.3] scsi_dh: use-after-free when removing scsi device Thread-Index: AQHQ++QJSjcN0F0J60adNPMWiOAr5p5Vd3MA Date: Thu, 1 Oct 2015 04:38:45 +0000 Message-ID: <20151001043844.GA8397@xzibit.linux.bs1.fc.nec.co.jp> References: <20150930003549.GA4857@xzibit.linux.bs1.fc.nec.co.jp> <20150930151857.GA26594@lst.de> <20151001005640.GA4821@xzibit.linux.bs1.fc.nec.co.jp> In-Reply-To: <20151001005640.GA4821@xzibit.linux.bs1.fc.nec.co.jp> Accept-Language: ja-JP, en-US Content-Language: ja-JP X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.34.125.85] Content-ID: MIME-Version: 1.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.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, 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 On 10/01/15 09:56, Junichi Nomura wrote: > With 4.2 kernel, scsi_dh->detach() was not called until the last > reference has gone. With 4.3-rc3, scsi_dh->detach() is directly called > from the context of scsi_remove_device(). That's the point. For my test script in the original post, I can't reproduce the crash by just swapping the order of scsi_dh_handler_detach() and device_remove_file() like this: void scsi_dh_remove_device(struct scsi_device *sdev) { device_remove_file(&sdev->sdev_gendev, &scsi_dh_state_attr); if (sdev->handler) scsi_dh_handler_detach(sdev); } But another workload using dm-multipath still caues the same crash. I think a patch like the following is needed. What do you think? The commit 1bab0de0274f ("dm-mpath, scsi_dh: don't let dm detach device handlers") removed reference counting of attached scsi device handler. As a result scsi_dh->detach() is directly called in the context of scsi_remove_device() where activation request can be still in flight. This patch moves scsi_dh_handler_detach() to scsi_device_dev_release_usercontext(), at that point the device is already in quiesced state. To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Acked-by: Christoph Hellwig diff --git a/drivers/scsi/scsi_dh.c b/drivers/scsi/scsi_dh.c index edb044a..19bf9db 100644 --- a/drivers/scsi/scsi_dh.c +++ b/drivers/scsi/scsi_dh.c @@ -232,10 +232,14 @@ int scsi_dh_add_device(struct scsi_device *sdev) return err; } -void scsi_dh_remove_device(struct scsi_device *sdev) +void scsi_dh_release_device(struct scsi_device *sdev) { if (sdev->handler) scsi_dh_handler_detach(sdev); +} + +void scsi_dh_remove_device(struct scsi_device *sdev) +{ device_remove_file(&sdev->sdev_gendev, &scsi_dh_state_attr); } diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h index 644bb73..4d01cdb 100644 --- a/drivers/scsi/scsi_priv.h +++ b/drivers/scsi/scsi_priv.h @@ -173,9 +173,11 @@ extern struct async_domain scsi_sd_probe_domain; /* scsi_dh.c */ #ifdef CONFIG_SCSI_DH int scsi_dh_add_device(struct scsi_device *sdev); +void scsi_dh_release_device(struct scsi_device *sdev); void scsi_dh_remove_device(struct scsi_device *sdev); #else static inline int scsi_dh_add_device(struct scsi_device *sdev) { return 0; } +static inline void scsi_dh_release_device(struct scsi_device *sdev) { } static inline void scsi_dh_remove_device(struct scsi_device *sdev) { } #endif diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index b333389..dff8faf 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c @@ -399,6 +399,8 @@ static void scsi_device_dev_release_usercontext(struct work_struct *work) sdev = container_of(work, struct scsi_device, ew.work); + scsi_dh_release_device(sdev); + parent = sdev->sdev_gendev.parent; spin_lock_irqsave(sdev->host->host_lock, flags);--