From nobody Mon Sep 29 21:22:41 2025 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 643DBC19F2C for ; Mon, 15 Aug 2022 22:20:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1347915AbiHOWUU (ORCPT ); Mon, 15 Aug 2022 18:20:20 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54872 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1348056AbiHOWQS (ORCPT ); Mon, 15 Aug 2022 18:16:18 -0400 Received: from sin.source.kernel.org (sin.source.kernel.org [IPv6:2604:1380:40e1:4800::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 68E3D120A9C; Mon, 15 Aug 2022 12:40:01 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sin.source.kernel.org (Postfix) with ESMTPS id 02E9FCE12E3; Mon, 15 Aug 2022 19:39:59 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id C55E2C433C1; Mon, 15 Aug 2022 19:39:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1660592397; bh=6iQ3c8g4QLzJzlAjpaRzfnF3Ohu+8PLvRf8ms+FV1Qg=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=mTEPk3zQaiJ33SH2wQWGuneyKaA0lVhBiDOdz614HZ6kCpIqyprSFIOD4q5xpTF7G vrQKR1iEJJpYzbN03l7LJruAdiBxINuuZsts4cbWLytfVKWm2S5DV3HKRiqMj8/042 vjVmF1d6xrUtUkiH6Teu6ggfo6ZPEErCwAvqECkw= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Douglas Gilbert , Tony Battersby , "Martin K. Petersen" Subject: [PATCH 5.19 0104/1157] scsi: sg: Allow waiting for commands to complete on removed device Date: Mon, 15 Aug 2022 19:51:00 +0200 Message-Id: <20220815180443.795089487@linuxfoundation.org> X-Mailer: git-send-email 2.37.2 In-Reply-To: <20220815180439.416659447@linuxfoundation.org> References: <20220815180439.416659447@linuxfoundation.org> User-Agent: quilt/0.67 MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Type: text/plain; charset="utf-8" From: Tony Battersby commit 3455607fd7be10b449f5135c00dc306b85dc0d21 upstream. When a SCSI device is removed while in active use, currently sg will immediately return -ENODEV on any attempt to wait for active commands that were sent before the removal. This is problematic for commands that use SG_FLAG_DIRECT_IO since the data buffer may still be in use by the kernel when userspace frees or reuses it after getting ENODEV, leading to corrupted userspace memory (in the case of READ-type commands) or corrupted data being sent to the device (in the case of WRITE-type commands). This has been seen in practice when logging out of a iscsi_tcp session, where the iSCSI driver may still be processing commands after the device has been marked for removal. Change the policy to allow userspace to wait for active sg commands even when the device is being removed. Return -ENODEV only when there are no more responses to read. Link: https://lore.kernel.org/r/5ebea46f-fe83-2d0b-233d-d0dcb362dd0a@cybern= etics.com Cc: Acked-by: Douglas Gilbert Signed-off-by: Tony Battersby Signed-off-by: Martin K. Petersen Signed-off-by: Greg Kroah-Hartman --- drivers/scsi/sg.c | 53 +++++++++++++++++++++++++++++++++----------------= ---- 1 file changed, 33 insertions(+), 20 deletions(-) --- a/drivers/scsi/sg.c +++ b/drivers/scsi/sg.c @@ -195,7 +195,7 @@ static void sg_link_reserve(Sg_fd * sfp, static void sg_unlink_reserve(Sg_fd * sfp, Sg_request * srp); static Sg_fd *sg_add_sfp(Sg_device * sdp); static void sg_remove_sfp(struct kref *); -static Sg_request *sg_get_rq_mark(Sg_fd * sfp, int pack_id); +static Sg_request *sg_get_rq_mark(Sg_fd * sfp, int pack_id, bool *busy); static Sg_request *sg_add_request(Sg_fd * sfp); static int sg_remove_request(Sg_fd * sfp, Sg_request * srp); static Sg_device *sg_get_dev(int dev); @@ -444,6 +444,7 @@ sg_read(struct file *filp, char __user * Sg_fd *sfp; Sg_request *srp; int req_pack_id =3D -1; + bool busy; sg_io_hdr_t *hp; struct sg_header *old_hdr; int retval; @@ -466,20 +467,16 @@ sg_read(struct file *filp, char __user * if (retval) return retval; =20 - srp =3D sg_get_rq_mark(sfp, req_pack_id); + srp =3D sg_get_rq_mark(sfp, req_pack_id, &busy); if (!srp) { /* now wait on packet to arrive */ - if (atomic_read(&sdp->detaching)) - return -ENODEV; if (filp->f_flags & O_NONBLOCK) return -EAGAIN; retval =3D wait_event_interruptible(sfp->read_wait, - (atomic_read(&sdp->detaching) || - (srp =3D sg_get_rq_mark(sfp, req_pack_id)))); - if (atomic_read(&sdp->detaching)) - return -ENODEV; - if (retval) - /* -ERESTARTSYS as signal hit process */ - return retval; + ((srp =3D sg_get_rq_mark(sfp, req_pack_id, &busy)) || + (!busy && atomic_read(&sdp->detaching)))); + if (!srp) + /* signal or detaching */ + return retval ? retval : -ENODEV; } if (srp->header.interface_id !=3D '\0') return sg_new_read(sfp, buf, count, srp); @@ -940,9 +937,7 @@ sg_ioctl_common(struct file *filp, Sg_de if (result < 0) return result; result =3D wait_event_interruptible(sfp->read_wait, - (srp_done(sfp, srp) || atomic_read(&sdp->detaching))); - if (atomic_read(&sdp->detaching)) - return -ENODEV; + srp_done(sfp, srp)); write_lock_irq(&sfp->rq_list_lock); if (srp->done) { srp->done =3D 2; @@ -2079,19 +2074,28 @@ sg_unlink_reserve(Sg_fd * sfp, Sg_reques } =20 static Sg_request * -sg_get_rq_mark(Sg_fd * sfp, int pack_id) +sg_get_rq_mark(Sg_fd * sfp, int pack_id, bool *busy) { Sg_request *resp; unsigned long iflags; =20 + *busy =3D false; write_lock_irqsave(&sfp->rq_list_lock, iflags); list_for_each_entry(resp, &sfp->rq_list, entry) { - /* look for requests that are ready + not SG_IO owned */ - if ((1 =3D=3D resp->done) && (!resp->sg_io_owned) && + /* look for requests that are not SG_IO owned */ + if ((!resp->sg_io_owned) && ((-1 =3D=3D pack_id) || (resp->header.pack_id =3D=3D pack_id))) { - resp->done =3D 2; /* guard against other readers */ - write_unlock_irqrestore(&sfp->rq_list_lock, iflags); - return resp; + switch (resp->done) { + case 0: /* request active */ + *busy =3D true; + break; + case 1: /* request done; response ready to return */ + resp->done =3D 2; /* guard against other readers */ + write_unlock_irqrestore(&sfp->rq_list_lock, iflags); + return resp; + case 2: /* response already being returned */ + break; + } } } write_unlock_irqrestore(&sfp->rq_list_lock, iflags); @@ -2145,6 +2149,15 @@ sg_remove_request(Sg_fd * sfp, Sg_reques res =3D 1; } write_unlock_irqrestore(&sfp->rq_list_lock, iflags); + + /* + * If the device is detaching, wakeup any readers in case we just + * removed the last response, which would leave nothing for them to + * return other than -ENODEV. + */ + if (unlikely(atomic_read(&sfp->parentdp->detaching))) + wake_up_interruptible_all(&sfp->read_wait); + return res; }