[SCSI] sg: prevent unwoken sleep

srp->done is protected by sfp->rq_list_lock everywhere, except for this
one case.  Result can be that the wake-up happens before the cacheline
with the changed srp->done has arrived, so the waiter can go back to
sleep and never be woken up again.

The wait_event_interruptible() means that anyone trying to debug this
unlikely race will likely notice everything working fine again, as the
next signal will unwedge things.  Evil.

Signed-off-by: Joern Engel <joern@logfs.org>
Acked-by: Douglas Gilbert <dgilbert@interlog.com>
Signed-off-by: James Bottomley <JBottomley@Parallels.com>

diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 511e3ca1afd64c453b19006dc720132f6a06d6a9..4a00364445f02ae8fc52411ed268f3b55f5277a3 100644 (file)
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -137,7 +137,8 @@ typedef struct sg_request { /* SG_MAX_QUEUE requests outstanding per file */
        char res_used;          /* 1 -> using reserve buffer, 0 -> not ... */
        char orphan;            /* 1 -> drop on sight, 0 -> normal */
        char sg_io_owned;       /* 1 -> packet belongs to SG_IO */
-       volatile char done;     /* 0->before bh, 1->before read, 2->read */
+       /* done protected by rq_list_lock */
+       char done;              /* 0->before bh, 1->before read, 2->read */
        struct request *rq;
        struct bio *bio;
        struct execute_work ew;
@@ -760,6 +761,17 @@ sg_common_write(Sg_fd * sfp, Sg_request * srp,
        return 0;
 }
 
+static int srp_done(Sg_fd *sfp, Sg_request *srp)
+{
+       unsigned long flags;
+       int ret;
+
+       read_lock_irqsave(&sfp->rq_list_lock, flags);
+       ret = srp->done;
+       read_unlock_irqrestore(&sfp->rq_list_lock, flags);
+       return ret;
+}
+
 static int
 sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg)
 {
@@ -791,7 +803,7 @@ sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg)
                if (result < 0)
                        return result;
                result = wait_event_interruptible(sfp->read_wait,
-                       (srp->done || sdp->detached));
+                       (srp_done(sfp, srp) || sdp->detached));
                if (sdp->detached)
                        return -ENODEV;
                write_lock_irq(&sfp->rq_list_lock);