lkml.org 
[lkml]   [2022]   [Aug]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.4 281/389] scsi: sg: Allow waiting for commands to complete on removed device
    Date
    From: Tony Battersby <tonyb@cybernetics.com>

    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@cybernetics.com
    Cc: <stable@vger.kernel.org>
    Acked-by: Douglas Gilbert <dgilbert@interlog.com>
    Signed-off-by: Tony Battersby <tonyb@cybernetics.com>
    Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    drivers/scsi/sg.c | 57 ++++++++++++++++++++++++++++++++----------------------
    1 file changed, 34 insertions(+), 23 deletions(-)

    --- a/drivers/scsi/sg.c
    +++ b/drivers/scsi/sg.c
    @@ -190,7 +190,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);
    @@ -412,6 +412,7 @@ sg_read(struct file *filp, char __user *
    Sg_fd *sfp;
    Sg_request *srp;
    int req_pack_id = -1;
    + bool busy;
    sg_io_hdr_t *hp;
    struct sg_header *old_hdr = NULL;
    int retval = 0;
    @@ -459,25 +460,19 @@ sg_read(struct file *filp, char __user *
    } else
    req_pack_id = old_hdr->pack_id;
    }
    - srp = sg_get_rq_mark(sfp, req_pack_id);
    + srp = sg_get_rq_mark(sfp, req_pack_id, &busy);
    if (!srp) { /* now wait on packet to arrive */
    - if (atomic_read(&sdp->detaching)) {
    - retval = -ENODEV;
    - goto free_old_hdr;
    - }
    if (filp->f_flags & O_NONBLOCK) {
    retval = -EAGAIN;
    goto free_old_hdr;
    }
    retval = wait_event_interruptible(sfp->read_wait,
    - (atomic_read(&sdp->detaching) ||
    - (srp = sg_get_rq_mark(sfp, req_pack_id))));
    - if (atomic_read(&sdp->detaching)) {
    - retval = -ENODEV;
    - goto free_old_hdr;
    - }
    - if (retval) {
    - /* -ERESTARTSYS as signal hit process */
    + ((srp = sg_get_rq_mark(sfp, req_pack_id, &busy)) ||
    + (!busy && atomic_read(&sdp->detaching))));
    + if (!srp) {
    + /* signal or detaching */
    + if (!retval)
    + retval = -ENODEV;
    goto free_old_hdr;
    }
    }
    @@ -928,9 +923,7 @@ sg_ioctl(struct file *filp, unsigned int
    if (result < 0)
    return result;
    result = 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 = 2;
    @@ -2074,19 +2067,28 @@ sg_unlink_reserve(Sg_fd * sfp, Sg_reques
    }

    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;

    + *busy = 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 == resp->done) && (!resp->sg_io_owned) &&
    + /* look for requests that are not SG_IO owned */
    + if ((!resp->sg_io_owned) &&
    ((-1 == pack_id) || (resp->header.pack_id == pack_id))) {
    - resp->done = 2; /* guard against other readers */
    - write_unlock_irqrestore(&sfp->rq_list_lock, iflags);
    - return resp;
    + switch (resp->done) {
    + case 0: /* request active */
    + *busy = true;
    + break;
    + case 1: /* request done; response ready to return */
    + resp->done = 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);
    @@ -2140,6 +2142,15 @@ sg_remove_request(Sg_fd * sfp, Sg_reques
    res = 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;
    }


    \
     
     \ /
      Last update: 2022-08-23 13:49    [W:2.717 / U:0.224 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site