From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 25D8BC43381 for ; Tue, 26 Mar 2019 01:56:32 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id E432C20811 for ; Tue, 26 Mar 2019 01:56:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730624AbfCZB4b (ORCPT ); Mon, 25 Mar 2019 21:56:31 -0400 Received: from mail-pg1-f195.google.com ([209.85.215.195]:45986 "EHLO mail-pg1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730533AbfCZB4b (ORCPT ); Mon, 25 Mar 2019 21:56:31 -0400 Received: by mail-pg1-f195.google.com with SMTP id y3so7457410pgk.12; Mon, 25 Mar 2019 18:56:30 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=CCMjTT6Heuh8neLmxE2XT2Qi8YX9ZuDWNPXta3vCV1k=; b=nAQtN2hmZxuQrlK2wrYJqLl5WBYN+2YAN65OAJDjjF/vwYwNng+3TMP9N3ftx+BeEd 45coAsm4spNCz+UT7j545Rutkb5IivI9DPk0Wht06eta7cr7PXMDbBh+eMt+vrMjgPxJ SxEuW6s1jY+S/f9QnRTULJgEDwEbCk425xD1Cv0x34kBFkfYquA2kScHxpIRk2rSPbIe ElzTOJko/RPror6MtsxKJXu+ntNCHhjufneDXs3hekDh/dt8CqetXdbz3Kix1/YRDZYN JQjT1/A0zhKck1YiYhGo1agJx8c+4pkF9Y07mQ3/7ZWPCqWlyC0zh+uool924nYwawg2 uHww== X-Gm-Message-State: APjAAAU40uXIS+iOcCZ8xfeDGIJDhL1HUyV0Dph4kqh0bpSRXsZXNBMj Lp+AnHeaztdDgwzjkhNoHyDT6iSOCfI= X-Google-Smtp-Source: APXvYqxCu38Twf4gnnFHpv5BOd6mGCdToluSRvFM2D5yvs+tXrgDAnFlyoWqNNZxy0Gii6QA0VQLug== X-Received: by 2002:a65:4bce:: with SMTP id p14mr26902976pgr.68.1553565389814; Mon, 25 Mar 2019 18:56:29 -0700 (PDT) Received: from asus.site ([2601:647:4000:5dd1:a41e:80b4:deb3:fb66]) by smtp.gmail.com with ESMTPSA id i5sm20317593pfd.16.2019.03.25.18.56.28 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 25 Mar 2019 18:56:29 -0700 (PDT) Subject: Re: [PATCH] sd: Fix a race between closing an sd device and sd I/O To: Ming Lei Cc: "Martin K . Petersen" , "James E . J . Bottomley" , linux-scsi@vger.kernel.org, Christoph Hellwig , Hannes Reinecke , Johannes Thumshirn , Jason Yan , stable@vger.kernel.org References: <20190325170146.184414-1-bvanassche@acm.org> <20190326014402.GD30669@ming.t460p> From: Bart Van Assche Message-ID: <939cd5ce-6a23-2f2b-616d-39be3440542d@acm.org> Date: Mon, 25 Mar 2019 18:56:28 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.5.2 MIME-Version: 1.0 In-Reply-To: <20190326014402.GD30669@ming.t460p> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: stable-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org On 3/25/19 6:44 PM, Ming Lei wrote: > On Mon, Mar 25, 2019 at 10:01:46AM -0700, Bart Van Assche wrote: >> The scsi_end_request() function calls scsi_cmd_to_driver() indirectly >> and hence needs the disk->private_data pointer. Avoid that that pointer >> is cleared before all affected I/O requests have finished. This patch >> avoids that the following crash occurs: >> >> Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000 >> Call trace: >> scsi_mq_uninit_cmd+0x1c/0x30 >> scsi_end_request+0x7c/0x1b8 >> scsi_io_completion+0x464/0x668 >> scsi_finish_command+0xbc/0x160 >> scsi_eh_flush_done_q+0x10c/0x170 >> sas_scsi_recover_host+0x84c/0xa98 [libsas] >> scsi_error_handler+0x140/0x5b0 >> kthread+0x100/0x12c >> ret_from_fork+0x10/0x18 >> >> Cc: Christoph Hellwig >> Cc: Ming Lei >> Cc: Hannes Reinecke >> Cc: Johannes Thumshirn >> Cc: Jason Yan >> Cc: >> Reported-by: Jason Yan >> Signed-off-by: Bart Van Assche >> --- >> drivers/scsi/sd.c | 19 +++++++++++++------ >> 1 file changed, 13 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c >> index ed34bfbc3844..0077880c0cc8 100644 >> --- a/drivers/scsi/sd.c >> +++ b/drivers/scsi/sd.c >> @@ -1416,11 +1416,6 @@ static void sd_release(struct gendisk *disk, fmode_t mode) >> scsi_set_medium_removal(sdev, SCSI_REMOVAL_ALLOW); >> } >> >> - /* >> - * XXX and what if there are packets in flight and this close() >> - * XXX is followed by a "rmmod sd_mod"? >> - */ >> - >> scsi_disk_put(sdkp); >> } >> >> @@ -3483,9 +3478,21 @@ static void scsi_disk_release(struct device *dev) >> { >> struct scsi_disk *sdkp = to_scsi_disk(dev); >> struct gendisk *disk = sdkp->disk; >> - >> + struct request_queue *q = disk->queue; >> + >> ida_free(&sd_index_ida, sdkp->index); >> >> + /* >> + * Wait until all requests that are in progress have completed. >> + * This is necessary to avoid that e.g. scsi_end_request() crashes >> + * due to clearing the disk->private_data pointer. Wait from inside >> + * scsi_disk_release() instead of from sd_release() to avoid that >> + * freezing and unfreezing the request queue affects user space I/O >> + * in case multiple processes open a /dev/sd... node concurrently. >> + */ >> + blk_mq_freeze_queue(q); >> + blk_mq_unfreeze_queue(q); >> + >> disk->private_data = NULL; >> put_disk(disk); >> put_device(&sdkp->device->sdev_gendev); > > No, this way may cause big performance issue, see my previous comment: > > https://marc.info/?l=linux-scsi&m=155321977714715&w=2 Have you had a look at this patch? Your comment applies to the previous version of this patch. I don't think that it applies to the current version. Bart.