From: Hans-Joachim Picht <hpicht@redhat.com> Date: Tue, 1 Jul 2008 15:12:46 +0200 Subject: [s390] fix race with stack local wait_queue_head_t. Message-id: 20080701131246.GC20922@redhat.com O-Subject: [RHEL5 U3 PATCH 3/6] s390 - fix race with stack local wait_queue_head_t. Bugzilla: 451279 RH-Acked-by: Pete Zaitcev <zaitcev@redhat.com> Description ============ A wait_event call with a stack local wait_queue_head_t structure that is used to do the wake up for the wait_event is inherently racy. After the wait_event finished the wake_up call might not have completed yet. Solution: Replace the stack local wait_queue_head_t in tape_do_io and tape_do_io_interruptible with a per device wait queue. The wait queues in dasd.c raw3270.c are replaced by a generic wait queue. Bugzilla ========= BZ 451279 https://bugzilla.redhat.com/show_bug.cgi?id=451279 Upstream status of the patch: ============================= The patch is included in linux-2.6 as git commit c80ee724966a8ce9a68020d9095233fb1c6f57e8 (dasd) 4657fb8a98a4e02981a574492bbe470c147b6657 (tape) 54ad64129cc166b9eec7151f3f9fc83589e33555 (3270) Upstream the fix for this problem is spread across three separate patches. Test status: ============ The patch has been tested and fixes the problem. The fix has been verified by the IBM test department. Please ACK. With best regards, -Hans diff --git a/drivers/s390/block/dasd.c b/drivers/s390/block/dasd.c index 59bca30..d83c1e1 100644 --- a/drivers/s390/block/dasd.c +++ b/drivers/s390/block/dasd.c @@ -61,6 +61,7 @@ static void do_kick_device(void *data); */ static wait_queue_head_t dasd_init_waitq; static wait_queue_head_t dasd_flush_wq; +static wait_queue_head_t generic_waitq; /* * Allocate memory for a new device structure. @@ -1549,16 +1550,14 @@ _wait_for_wakeup(struct dasd_ccw_req *cqr) int dasd_sleep_on(struct dasd_ccw_req * cqr) { - wait_queue_head_t wait_q; struct dasd_device *device; int rc; device = cqr->device; spin_lock_irq(get_ccwdev_lock(device->cdev)); - init_waitqueue_head (&wait_q); cqr->callback = dasd_wakeup_cb; - cqr->callback_data = (void *) &wait_q; + cqr->callback_data = (void *) &generic_waitq; cqr->status = DASD_CQR_QUEUED; list_add_tail(&cqr->list, &device->ccw_queue); @@ -1567,7 +1566,7 @@ dasd_sleep_on(struct dasd_ccw_req * cqr) spin_unlock_irq(get_ccwdev_lock(device->cdev)); - wait_event(wait_q, _wait_for_wakeup(cqr)); + wait_event(generic_waitq, _wait_for_wakeup(cqr)); /* Request status is either done or failed. */ rc = (cqr->status == DASD_CQR_FAILED) ? -EIO : 0; @@ -1581,16 +1580,14 @@ dasd_sleep_on(struct dasd_ccw_req * cqr) int dasd_sleep_on_interruptible(struct dasd_ccw_req * cqr) { - wait_queue_head_t wait_q; struct dasd_device *device; int rc, finished; device = cqr->device; spin_lock_irq(get_ccwdev_lock(device->cdev)); - init_waitqueue_head (&wait_q); cqr->callback = dasd_wakeup_cb; - cqr->callback_data = (void *) &wait_q; + cqr->callback_data = (void *) &generic_waitq; cqr->status = DASD_CQR_QUEUED; list_add_tail(&cqr->list, &device->ccw_queue); @@ -1600,7 +1597,8 @@ dasd_sleep_on_interruptible(struct dasd_ccw_req * cqr) finished = 0; while (!finished) { - rc = wait_event_interruptible(wait_q, _wait_for_wakeup(cqr)); + rc = wait_event_interruptible(generic_waitq, + _wait_for_wakeup(cqr)); if (rc != -ERESTARTSYS) { /* Request is final (done or failed) */ rc = (cqr->status == DASD_CQR_DONE) ? 0 : -EIO; @@ -1616,7 +1614,8 @@ dasd_sleep_on_interruptible(struct dasd_ccw_req * cqr) /* wait (non-interruptible) for final status * because signal ist still pending */ spin_unlock_irq(get_ccwdev_lock(device->cdev)); - wait_event(wait_q, _wait_for_wakeup(cqr)); + wait_event(generic_waitq, + _wait_for_wakeup(cqr)); spin_lock_irq(get_ccwdev_lock(device->cdev)); rc = (cqr->status == DASD_CQR_DONE) ? 0 : -EIO; finished = 1; @@ -1680,7 +1679,7 @@ dasd_sleep_on_immediatly(struct dasd_ccw_req * cqr) spin_unlock_irq(get_ccwdev_lock(device->cdev)); - wait_event(wait_q, _wait_for_wakeup(cqr)); + wait_event(generic_waitq, _wait_for_wakeup(cqr)); /* Request status is either done or failed. */ rc = (cqr->status == DASD_CQR_FAILED) ? -EIO : 0; @@ -2183,6 +2182,7 @@ dasd_init(void) init_waitqueue_head(&dasd_init_waitq); init_waitqueue_head(&dasd_flush_wq); + init_waitqueue_head(&generic_waitq); /* register 'common' DASD debug area, used for all DBF_XXX calls */ dasd_debug_area = debug_register("dasd", 1, 2, 8 * sizeof (long)); diff --git a/drivers/s390/char/raw3270.c b/drivers/s390/char/raw3270.c index d628620..16eba4e 100644 --- a/drivers/s390/char/raw3270.c +++ b/drivers/s390/char/raw3270.c @@ -550,7 +550,6 @@ raw3270_start_init(struct raw3270 *rp, struct raw3270_view *view, struct raw3270_request *rq) { unsigned long flags; - wait_queue_head_t wq; int rc; #ifdef CONFIG_TN3270_CONSOLE @@ -567,20 +566,20 @@ raw3270_start_init(struct raw3270 *rp, struct raw3270_view *view, return rq->rc; } #endif - init_waitqueue_head(&wq); rq->callback = raw3270_wake_init; - rq->callback_data = &wq; + rq->callback_data = &raw3270_wait_queue; spin_lock_irqsave(get_ccwdev_lock(view->dev->cdev), flags); rc = __raw3270_start(rp, view, rq); spin_unlock_irqrestore(get_ccwdev_lock(view->dev->cdev), flags); if (rc) return rc; /* Now wait for the completion. */ - rc = wait_event_interruptible(wq, raw3270_request_final(rq)); + rc = wait_event_interruptible(raw3270_wait_queue, + raw3270_request_final(rq)); if (rc == -ERESTARTSYS) { /* Interrupted by a signal. */ raw3270_halt_io(view->dev, rq); /* No wait for the halt to complete. */ - wait_event(wq, raw3270_request_final(rq)); + wait_event(raw3270_wait_queue, raw3270_request_final(rq)); return -ERESTARTSYS; } return rq->rc; diff --git a/drivers/s390/char/tape.h b/drivers/s390/char/tape.h index 5f510e5..e208e1b 100644 --- a/drivers/s390/char/tape.h +++ b/drivers/s390/char/tape.h @@ -229,6 +229,9 @@ struct tape_device { /* Request queue. */ struct list_head req_queue; + /* Request wait queue. */ + wait_queue_head_t wait_queue; + /* Each tape device has (currently) two minor numbers. */ int first_minor; diff --git a/drivers/s390/char/tape_core.c b/drivers/s390/char/tape_core.c index 7f144f5..c72ed1f 100644 --- a/drivers/s390/char/tape_core.c +++ b/drivers/s390/char/tape_core.c @@ -480,6 +480,7 @@ tape_alloc_device(void) INIT_LIST_HEAD(&device->req_queue); INIT_LIST_HEAD(&device->node); init_waitqueue_head(&device->state_change_wq); + init_waitqueue_head(&device->wait_queue); device->tape_state = TS_INIT; device->medium_state = MS_UNKNOWN; *device->modeset_byte = 0; @@ -1002,21 +1003,19 @@ __tape_wake_up(struct tape_request *request, void *data) int tape_do_io(struct tape_device *device, struct tape_request *request) { - wait_queue_head_t wq; int rc; - init_waitqueue_head(&wq); spin_lock_irq(get_ccwdev_lock(device->cdev)); /* Setup callback */ request->callback = __tape_wake_up; - request->callback_data = &wq; + request->callback_data = &device->wait_queue; /* Add request to request queue and try to start it. */ rc = __tape_start_request(device, request); spin_unlock_irq(get_ccwdev_lock(device->cdev)); if (rc) return rc; /* Request added to the queue. Wait for its completion. */ - wait_event(wq, (request->callback == NULL)); + wait_event(device->wait_queue, (request->callback == NULL)); /* Get rc from request */ return request->rc; } @@ -1037,20 +1036,19 @@ int tape_do_io_interruptible(struct tape_device *device, struct tape_request *request) { - wait_queue_head_t wq; int rc; - init_waitqueue_head(&wq); spin_lock_irq(get_ccwdev_lock(device->cdev)); /* Setup callback */ request->callback = __tape_wake_up_interruptible; - request->callback_data = &wq; + request->callback_data = &device->wait_queue; rc = __tape_start_request(device, request); spin_unlock_irq(get_ccwdev_lock(device->cdev)); if (rc) return rc; /* Request added to the queue. Wait for its completion. */ - rc = wait_event_interruptible(wq, (request->callback == NULL)); + rc = wait_event_interruptible(device->wait_queue, + (request->callback == NULL)); if (rc != -ERESTARTSYS) /* Request finished normally. */ return request->rc; @@ -1063,7 +1061,7 @@ tape_do_io_interruptible(struct tape_device *device, /* Wait for the interrupt that acknowledges the halt. */ do { rc = wait_event_interruptible( - wq, + device->wait_queue, (request->callback == NULL) ); } while (rc == -ERESTARTSYS);