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=-8.5 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED, USER_AGENT_MUTT 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 63BDBC43381 for ; Tue, 26 Mar 2019 01:44:17 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 3BF34206DF for ; Tue, 26 Mar 2019 01:44:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730804AbfCZBoQ (ORCPT ); Mon, 25 Mar 2019 21:44:16 -0400 Received: from mx1.redhat.com ([209.132.183.28]:41234 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730061AbfCZBoQ (ORCPT ); Mon, 25 Mar 2019 21:44:16 -0400 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 0897BC0B6174; Tue, 26 Mar 2019 01:44:16 +0000 (UTC) Received: from ming.t460p (ovpn-8-21.pek2.redhat.com [10.72.8.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id A7CD2642BD; Tue, 26 Mar 2019 01:44:08 +0000 (UTC) Date: Tue, 26 Mar 2019 09:44:03 +0800 From: Ming Lei To: Bart Van Assche 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 Subject: Re: [PATCH] sd: Fix a race between closing an sd device and sd I/O Message-ID: <20190326014402.GD30669@ming.t460p> References: <20190325170146.184414-1-bvanassche@acm.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190325170146.184414-1-bvanassche@acm.org> User-Agent: Mutt/1.9.1 (2017-09-22) X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Tue, 26 Mar 2019 01:44:16 +0000 (UTC) Sender: stable-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org 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 Thanks, Ming