diff options
author | Tony Battersby <tonyb@cybernetics.com> | 2009-01-20 22:00:09 (GMT) |
---|---|---|
committer | James Bottomley <James.Bottomley@HansenPartnership.com> | 2009-03-12 17:58:05 (GMT) |
commit | a2dd3b4cea335713b58996bb07b3abcde1175f47 (patch) | |
tree | 3097816f8abd5fb57dda669d3e776c35f1455f43 /drivers/scsi/sg.c | |
parent | c6517b7942fad663cc1cf3235cbe4207cf769332 (diff) | |
download | linux-a2dd3b4cea335713b58996bb07b3abcde1175f47.tar.xz |
[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>
Diffstat (limited to 'drivers/scsi/sg.c')
-rw-r--r-- | drivers/scsi/sg.c | 39 |
1 files changed, 14 insertions, 25 deletions
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c index b447527..18d079e 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; } |