diff options
author | James Bottomley <JBottomley@Parallels.com> | 2013-10-25 05:27:02 -0400 |
---|---|---|
committer | James Bottomley <JBottomley@Parallels.com> | 2013-10-25 05:59:54 -0400 |
commit | 065b4a2f5952df2c46aa04d24ffcce65cc75a1a9 (patch) | |
tree | d78a56cbc1941487f68b227b9bf6066faa9419b7 | |
parent | 98481ff0bb8792ebfb832e330e56d3c629ba5fa6 (diff) |
[SCSI] Revert "sg: use rwsem to solve race during exclusive open"
This reverts commit 15b06f9a02406e5460001db6d5af5c738cd3d4e7.
This is one of four patches that was causing this bug
[ 205.372823] ================================================
[ 205.372901] [ BUG: lock held when returning to user space! ]
[ 205.372979] 3.12.0-rc6-hw-debug-pagealloc+ #67 Not tainted
[ 205.373055] ------------------------------------------------
[ 205.373132] megarc.bin/5283 is leaving the kernel with locks still held!
[ 205.373212] 1 lock held by megarc.bin/5283:
[ 205.373285] #0: (&sdp->o_sem){.+.+..}, at: [<ffffffff8161e650>] sg_open+0x3a0/0x4d0
Cc: Vaughan Cao <vaughan.cao@oracle.com>
Acked-by: Douglas Gilbert <dgilbert@interlog.com>
Signed-off-by: James Bottomley <JBottomley@Parallels.com>
-rw-r--r-- | drivers/scsi/sg.c | 79 |
1 files changed, 38 insertions, 41 deletions
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c index 4efa9b5884b7..df5e961484e1 100644 --- a/drivers/scsi/sg.c +++ b/drivers/scsi/sg.c | |||
@@ -170,11 +170,11 @@ typedef struct sg_fd { /* holds the state of a file descriptor */ | |||
170 | 170 | ||
171 | typedef struct sg_device { /* holds the state of each scsi generic device */ | 171 | typedef struct sg_device { /* holds the state of each scsi generic device */ |
172 | struct scsi_device *device; | 172 | struct scsi_device *device; |
173 | wait_queue_head_t o_excl_wait; /* queue open() when O_EXCL in use */ | ||
173 | int sg_tablesize; /* adapter's max scatter-gather table size */ | 174 | int sg_tablesize; /* adapter's max scatter-gather table size */ |
174 | u32 index; /* device index number */ | 175 | u32 index; /* device index number */ |
175 | /* sfds is protected by sg_index_lock */ | 176 | /* sfds is protected by sg_index_lock */ |
176 | struct list_head sfds; | 177 | struct list_head sfds; |
177 | struct rw_semaphore o_sem; /* exclude open should hold this rwsem */ | ||
178 | volatile char detached; /* 0->attached, 1->detached pending removal */ | 178 | volatile char detached; /* 0->attached, 1->detached pending removal */ |
179 | /* exclude protected by sg_open_exclusive_lock */ | 179 | /* exclude protected by sg_open_exclusive_lock */ |
180 | char exclude; /* opened for exclusive access */ | 180 | char exclude; /* opened for exclusive access */ |
@@ -265,6 +265,7 @@ sg_open(struct inode *inode, struct file *filp) | |||
265 | struct request_queue *q; | 265 | struct request_queue *q; |
266 | Sg_device *sdp; | 266 | Sg_device *sdp; |
267 | Sg_fd *sfp; | 267 | Sg_fd *sfp; |
268 | int res; | ||
268 | int retval; | 269 | int retval; |
269 | 270 | ||
270 | nonseekable_open(inode, filp); | 271 | nonseekable_open(inode, filp); |
@@ -293,35 +294,35 @@ sg_open(struct inode *inode, struct file *filp) | |||
293 | goto error_out; | 294 | goto error_out; |
294 | } | 295 | } |
295 | 296 | ||
296 | if ((flags & O_EXCL) && (O_RDONLY == (flags & O_ACCMODE))) { | 297 | if (flags & O_EXCL) { |
297 | retval = -EPERM; /* Can't lock it with read only access */ | 298 | if (O_RDONLY == (flags & O_ACCMODE)) { |
298 | goto error_out; | 299 | retval = -EPERM; /* Can't lock it with read only access */ |
299 | } | 300 | goto error_out; |
300 | if (flags & O_NONBLOCK) { | 301 | } |
301 | if (flags & O_EXCL) { | 302 | if (!sfds_list_empty(sdp) && (flags & O_NONBLOCK)) { |
302 | if (!down_write_trylock(&sdp->o_sem)) { | 303 | retval = -EBUSY; |
303 | retval = -EBUSY; | 304 | goto error_out; |
304 | goto error_out; | 305 | } |
305 | } | 306 | res = wait_event_interruptible(sdp->o_excl_wait, |
306 | } else { | 307 | ((!sfds_list_empty(sdp) || get_exclude(sdp)) ? 0 : set_exclude(sdp, 1))); |
307 | if (!down_read_trylock(&sdp->o_sem)) { | 308 | if (res) { |
308 | retval = -EBUSY; | 309 | retval = res; /* -ERESTARTSYS because signal hit process */ |
309 | goto error_out; | 310 | goto error_out; |
310 | } | 311 | } |
312 | } else if (get_exclude(sdp)) { /* some other fd has an exclusive lock on dev */ | ||
313 | if (flags & O_NONBLOCK) { | ||
314 | retval = -EBUSY; | ||
315 | goto error_out; | ||
316 | } | ||
317 | res = wait_event_interruptible(sdp->o_excl_wait, !get_exclude(sdp)); | ||
318 | if (res) { | ||
319 | retval = res; /* -ERESTARTSYS because signal hit process */ | ||
320 | goto error_out; | ||
311 | } | 321 | } |
312 | } else { | ||
313 | if (flags & O_EXCL) | ||
314 | down_write(&sdp->o_sem); | ||
315 | else | ||
316 | down_read(&sdp->o_sem); | ||
317 | } | 322 | } |
318 | /* Since write lock is held, no need to check sfd_list */ | ||
319 | if (flags & O_EXCL) | ||
320 | set_exclude(sdp, 1); | ||
321 | |||
322 | if (sdp->detached) { | 323 | if (sdp->detached) { |
323 | retval = -ENODEV; | 324 | retval = -ENODEV; |
324 | goto sem_out; | 325 | goto error_out; |
325 | } | 326 | } |
326 | if (sfds_list_empty(sdp)) { /* no existing opens on this device */ | 327 | if (sfds_list_empty(sdp)) { /* no existing opens on this device */ |
327 | sdp->sgdebug = 0; | 328 | sdp->sgdebug = 0; |
@@ -330,18 +331,17 @@ sg_open(struct inode *inode, struct file *filp) | |||
330 | } | 331 | } |
331 | if ((sfp = sg_add_sfp(sdp, dev))) | 332 | if ((sfp = sg_add_sfp(sdp, dev))) |
332 | filp->private_data = sfp; | 333 | filp->private_data = sfp; |
333 | /* retval is already provably zero at this point because of the | ||
334 | * check after retval = scsi_autopm_get_device(sdp->device)) | ||
335 | */ | ||
336 | else { | 334 | else { |
337 | retval = -ENOMEM; | ||
338 | sem_out: | ||
339 | if (flags & O_EXCL) { | 335 | if (flags & O_EXCL) { |
340 | set_exclude(sdp, 0); /* undo if error */ | 336 | set_exclude(sdp, 0); /* undo if error */ |
341 | up_write(&sdp->o_sem); | 337 | wake_up_interruptible(&sdp->o_excl_wait); |
342 | } else | 338 | } |
343 | up_read(&sdp->o_sem); | 339 | retval = -ENOMEM; |
340 | goto error_out; | ||
341 | } | ||
342 | retval = 0; | ||
344 | error_out: | 343 | error_out: |
344 | if (retval) { | ||
345 | scsi_autopm_put_device(sdp->device); | 345 | scsi_autopm_put_device(sdp->device); |
346 | sdp_put: | 346 | sdp_put: |
347 | scsi_device_put(sdp->device); | 347 | scsi_device_put(sdp->device); |
@@ -358,18 +358,13 @@ sg_release(struct inode *inode, struct file *filp) | |||
358 | { | 358 | { |
359 | Sg_device *sdp; | 359 | Sg_device *sdp; |
360 | Sg_fd *sfp; | 360 | Sg_fd *sfp; |
361 | int excl; | ||
362 | 361 | ||
363 | if ((!(sfp = (Sg_fd *) filp->private_data)) || (!(sdp = sfp->parentdp))) | 362 | if ((!(sfp = (Sg_fd *) filp->private_data)) || (!(sdp = sfp->parentdp))) |
364 | return -ENXIO; | 363 | return -ENXIO; |
365 | SCSI_LOG_TIMEOUT(3, printk("sg_release: %s\n", sdp->disk->disk_name)); | 364 | SCSI_LOG_TIMEOUT(3, printk("sg_release: %s\n", sdp->disk->disk_name)); |
366 | 365 | ||
367 | excl = get_exclude(sdp); | ||
368 | set_exclude(sdp, 0); | 366 | set_exclude(sdp, 0); |
369 | if (excl) | 367 | wake_up_interruptible(&sdp->o_excl_wait); |
370 | up_write(&sdp->o_sem); | ||
371 | else | ||
372 | up_read(&sdp->o_sem); | ||
373 | 368 | ||
374 | scsi_autopm_put_device(sdp->device); | 369 | scsi_autopm_put_device(sdp->device); |
375 | kref_put(&sfp->f_ref, sg_remove_sfp); | 370 | kref_put(&sfp->f_ref, sg_remove_sfp); |
@@ -1421,7 +1416,7 @@ static Sg_device *sg_alloc(struct gendisk *disk, struct scsi_device *scsidp) | |||
1421 | sdp->disk = disk; | 1416 | sdp->disk = disk; |
1422 | sdp->device = scsidp; | 1417 | sdp->device = scsidp; |
1423 | INIT_LIST_HEAD(&sdp->sfds); | 1418 | INIT_LIST_HEAD(&sdp->sfds); |
1424 | init_rwsem(&sdp->o_sem); | 1419 | init_waitqueue_head(&sdp->o_excl_wait); |
1425 | sdp->sg_tablesize = queue_max_segments(q); | 1420 | sdp->sg_tablesize = queue_max_segments(q); |
1426 | sdp->index = k; | 1421 | sdp->index = k; |
1427 | kref_init(&sdp->d_ref); | 1422 | kref_init(&sdp->d_ref); |
@@ -2132,11 +2127,13 @@ static void sg_remove_sfp_usercontext(struct work_struct *work) | |||
2132 | static void sg_remove_sfp(struct kref *kref) | 2127 | static void sg_remove_sfp(struct kref *kref) |
2133 | { | 2128 | { |
2134 | struct sg_fd *sfp = container_of(kref, struct sg_fd, f_ref); | 2129 | struct sg_fd *sfp = container_of(kref, struct sg_fd, f_ref); |
2130 | struct sg_device *sdp = sfp->parentdp; | ||
2135 | unsigned long iflags; | 2131 | unsigned long iflags; |
2136 | 2132 | ||
2137 | write_lock_irqsave(&sg_index_lock, iflags); | 2133 | write_lock_irqsave(&sg_index_lock, iflags); |
2138 | list_del(&sfp->sfd_siblings); | 2134 | list_del(&sfp->sfd_siblings); |
2139 | write_unlock_irqrestore(&sg_index_lock, iflags); | 2135 | write_unlock_irqrestore(&sg_index_lock, iflags); |
2136 | wake_up_interruptible(&sdp->o_excl_wait); | ||
2140 | 2137 | ||
2141 | INIT_WORK(&sfp->ew.work, sg_remove_sfp_usercontext); | 2138 | INIT_WORK(&sfp->ew.work, sg_remove_sfp_usercontext); |
2142 | schedule_work(&sfp->ew.work); | 2139 | schedule_work(&sfp->ew.work); |