Commit a2dd3b4c authored by Tony Battersby's avatar Tony Battersby Committed by James Bottomley

[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: default avatarTony Battersby <tonyb@cybernetics.com>
Acked-by: default avatarDouglas Gilbert <dgilbert@interlog.com>
Signed-off-by: default avatarJames Bottomley <James.Bottomley@HansenPartnership.com>
parent c6517b79
...@@ -189,7 +189,7 @@ static ssize_t sg_new_read(Sg_fd * sfp, char __user *buf, size_t count, ...@@ -189,7 +189,7 @@ static ssize_t sg_new_read(Sg_fd * sfp, char __user *buf, size_t count,
Sg_request * srp); Sg_request * srp);
static ssize_t sg_new_write(Sg_fd *sfp, struct file *file, static ssize_t sg_new_write(Sg_fd *sfp, struct file *file,
const char __user *buf, size_t count, int blocking, 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, static int sg_common_write(Sg_fd * sfp, Sg_request * srp,
unsigned char *cmnd, int timeout, int blocking); unsigned char *cmnd, int timeout, int blocking);
static int sg_read_oxfer(Sg_request * srp, char __user *outp, int num_read_xfer); 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) ...@@ -561,7 +561,8 @@ sg_write(struct file *filp, const char __user *buf, size_t count, loff_t * ppos)
return -EFAULT; return -EFAULT;
blocking = !(filp->f_flags & O_NONBLOCK); blocking = !(filp->f_flags & O_NONBLOCK);
if (old_hdr.reply_len < 0) 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)) if (count < (SZ_SG_HEADER + 6))
return -EIO; /* The minimum scsi command length is 6 bytes. */ 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) ...@@ -642,7 +643,7 @@ sg_write(struct file *filp, const char __user *buf, size_t count, loff_t * ppos)
static ssize_t static ssize_t
sg_new_write(Sg_fd *sfp, struct file *file, const char __user *buf, 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) Sg_request **o_srp)
{ {
int k; int k;
...@@ -662,6 +663,7 @@ sg_new_write(Sg_fd *sfp, struct file *file, const char __user *buf, ...@@ -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")); SCSI_LOG_TIMEOUT(1, printk("sg_new_write: queue full\n"));
return -EDOM; return -EDOM;
} }
srp->sg_io_owned = sg_io_owned;
hp = &srp->header; hp = &srp->header;
if (__copy_from_user(hp, buf, SZ_SG_IO_HDR)) { if (__copy_from_user(hp, buf, SZ_SG_IO_HDR)) {
sg_remove_request(sfp, srp); sg_remove_request(sfp, srp);
...@@ -765,18 +767,6 @@ sg_common_write(Sg_fd * sfp, Sg_request * srp, ...@@ -765,18 +767,6 @@ sg_common_write(Sg_fd * sfp, Sg_request * srp,
return 0; return 0;
} }
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 static int
sg_ioctl(struct inode *inode, struct file *filp, sg_ioctl(struct inode *inode, struct file *filp,
unsigned int cmd_in, unsigned long arg) unsigned int cmd_in, unsigned long arg)
...@@ -809,27 +799,26 @@ sg_ioctl(struct inode *inode, struct file *filp, ...@@ -809,27 +799,26 @@ sg_ioctl(struct inode *inode, struct file *filp,
return -EFAULT; return -EFAULT;
result = result =
sg_new_write(sfp, filp, p, SZ_SG_IO_HDR, sg_new_write(sfp, filp, p, SZ_SG_IO_HDR,
blocking, read_only, &srp); blocking, read_only, 1, &srp);
if (result < 0) if (result < 0)
return result; return result;
srp->sg_io_owned = 1;
while (1) { while (1) {
result = 0; /* following macro to beat race condition */ result = 0; /* following macro to beat race condition */
__wait_event_interruptible(sfp->read_wait, __wait_event_interruptible(sfp->read_wait,
(sdp->detached || sfp->closed || sg_srp_done(srp, sfp)), (srp->done || sdp->detached),
result); result);
if (sdp->detached) if (sdp->detached)
return -ENODEV; return -ENODEV;
if (sfp->closed) write_lock_irq(&sfp->rq_list_lock);
return 0; /* request packet dropped already */ if (srp->done) {
if (0 == result) srp->done = 2;
write_unlock_irq(&sfp->rq_list_lock);
break; break;
}
srp->orphan = 1; srp->orphan = 1;
write_unlock_irq(&sfp->rq_list_lock);
return result; /* -ERESTARTSYS because signal hit process */ 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); result = sg_new_read(sfp, p, SZ_SG_IO_HDR, srp);
return (result < 0) ? result : 0; return (result < 0) ? result : 0;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment