[SCSI] sg: fix races with ioctl(SG_IO)

sg_io_owned needs to be set before the command is sent to the midlevel;
otherwise, a quickly-completing command may cause a different CPU
to see "srp->done == 1 && !srp->sg_io_owned", which would lead to
incorrect behavior.

Check srp->done and set srp->orphan while holding rq_list_lock to
prevent races with sg_rq_end_io().

There is no need to check sfp->closed from read/write/ioctl/poll/etc.
since the kernel guarantees that this won't happen.

The usefulness of sg_srp_done() was questionable before; now it is
definitely not needed.

Signed-off-by: Tony Battersby <tonyb@cybernetics.com>
Acked-by: Douglas Gilbert <dgilbert@interlog.com>
Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>

1 files changed, 14 insertions, 25 deletions

diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index b447527555a7..18d079e3990e 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -189,7 +189,7 @@ static ssize_t sg_new_read(Sg_fd * sfp, char __user *buf, size_t count,
                            Sg_request * srp);
 static ssize_t sg_new_write(Sg_fd *sfp, struct file *file,
                         const char __user *buf, size_t count, int blocking,
-                        int read_only, Sg_request **o_srp);
+                        int read_only, int sg_io_owned, Sg_request **o_srp);
 static int sg_common_write(Sg_fd * sfp, Sg_request * srp,
                            unsigned char *cmnd, int timeout, int blocking);
 static int sg_read_oxfer(Sg_request * srp, char __user *outp, int num_read_xfer);
@@ -561,7 +561,8 @@ sg_write(struct file *filp, const char __user *buf, size_t count, loff_t * ppos)
                 return -EFAULT;
         blocking = !(filp->f_flags & O_NONBLOCK);
         if (old_hdr.reply_len < 0)
-                return sg_new_write(sfp, filp, buf, count, blocking, 0, NULL);
+                return sg_new_write(sfp, filp, buf, count,
+                                    blocking, 0, 0, NULL);
         if (count < (SZ_SG_HEADER + 6))
                 return -EIO;    /* The minimum scsi command length is 6 bytes. */
 
@@ -642,7 +643,7 @@ sg_write(struct file *filp, const char __user *buf, size_t count, loff_t * ppos)
 
 static ssize_t
 sg_new_write(Sg_fd *sfp, struct file *file, const char __user *buf,
-                 size_t count, int blocking, int read_only,
+                 size_t count, int blocking, int read_only, int sg_io_owned,
                  Sg_request **o_srp)
 {
         int k;
@@ -662,6 +663,7 @@ sg_new_write(Sg_fd *sfp, struct file *file, const char __user *buf,
                 SCSI_LOG_TIMEOUT(1, printk("sg_new_write: queue full\n"));
                 return -EDOM;
         }
+        srp->sg_io_owned = sg_io_owned;
         hp = &srp->header;
         if (__copy_from_user(hp, buf, SZ_SG_IO_HDR)) {
                 sg_remove_request(sfp, srp);
@@ -766,18 +768,6 @@ sg_common_write(Sg_fd * sfp, Sg_request * srp,
 }
 
 static int
-sg_srp_done(Sg_request *srp, Sg_fd *sfp)
-{
-        unsigned long iflags;
-        int done;
-
-        read_lock_irqsave(&sfp->rq_list_lock, iflags);
-        done = srp->done;
-        read_unlock_irqrestore(&sfp->rq_list_lock, iflags);
-        return done;
-}
-
-static int
 sg_ioctl(struct inode *inode, struct file *filp,
          unsigned int cmd_in, unsigned long arg)
 {
@@ -809,27 +799,26 @@ sg_ioctl(struct inode *inode, struct file *filp,
                                 return -EFAULT;
                         result =
                             sg_new_write(sfp, filp, p, SZ_SG_IO_HDR,
-                                         blocking, read_only, &srp);
+                                         blocking, read_only, 1, &srp);
                         if (result < 0)
                                 return result;
-                        srp->sg_io_owned = 1;
                         while (1) {
                                 result = 0;     /* following macro to beat race condition */
                                 __wait_event_interruptible(sfp->read_wait,
-                                        (sdp->detached || sfp->closed || sg_srp_done(srp, sfp)),
-                                                           result);
+                                        (srp->done || sdp->detached),
+                                        result);
                                 if (sdp->detached)
                                         return -ENODEV;
-                                if (sfp->closed)
-                                        return 0;       /* request packet dropped already */
-                                if (0 == result)
+                                write_lock_irq(&sfp->rq_list_lock);
+                                if (srp->done) {
+                                        srp->done = 2;
+                                        write_unlock_irq(&sfp->rq_list_lock);
                                         break;
+                                }
                                 srp->orphan = 1;
+                                write_unlock_irq(&sfp->rq_list_lock);
                                 return result;  /* -ERESTARTSYS because signal hit process */
                         }
-                        write_lock_irqsave(&sfp->rq_list_lock, iflags);
-                        srp->done = 2;
-                        write_unlock_irqrestore(&sfp->rq_list_lock, iflags);
                         result = sg_new_read(sfp, p, SZ_SG_IO_HDR, srp);
                         return (result < 0) ? result : 0;
                 }