: Fix erroneous rq->buffer = NULL in ide-io.c:ide_dma_timeout_retry
PATCH 1/1 : Fix erroneous rq->buffer = NULL in ide-io.c:ide_dma_timeout_retry
Hello,
I have found an IDE bug in the IDE DMA timeout function, ide-io.c: ide_dma_timeout_retry erroneously sets the first_rq->buffer = NULL.
ide_dma_timeout_retry will be called whenever a command is issued, times out, and the drive is waiting for DMA. The function, ide_dma_timeout_retry, un-busies the hardware group and attempts to clean up the current request. As part of this cleanup the current failed first_rq->buffer is set to NULL.
However, as part of this retry process first_rq is retried up to 3 times in PIO mode (with DMA off).
During the retry, ide-cd.c: cdrom_start_read is called, which in turn calls, restore_request. restore_request references first_rq->buffer (which is NULL) in order to calculate hard_cur_sectors, hard_nr_sectors, etc.
ie) All of these values will be bogus because of the first_rq->buffer = NULL.
This request will fail and the IDE core will enter error handling. IDE core generates a new request, sense_rq, in order to request sense. Attached this request is a back pointer to the original first_rq request.
ie) sense_rq->buffer = first_rq
Eventually ide-cd.c:cdrom_end_request is called on sense_rq, and then ide-io.c:ide_end_dequeued_request is called on first_rq. Note that ide_end_dequeued_request is called with the bogus values from first_rq.
The return value essentially depends on the return value of ll_rw_blk.c:__end_that_request_first. The arguements to this function include nr_sectors, which as noted above, is bogus.
This leads to a return of 1 from ll_rw_blk.c:__end_that_request_first which eventually leads to an erroneous call to BUG() in ide-cd.c:cdrom_end_request.
I have forced this issue to occur by modifying code to effectively DMA timeout on CDROM accesses on i686 and ia64 platforms. I hit the bug 100% of the time.
It appears that the modification should be to rid the ide-io.c code of the rq->buffer = NULL call.
Patch is based off of latest BK linux-2.5 as of 2005-01-04 09:00.
drivers/ide/ide-io.c 2005-01-04 09:32:23.000000000 -0500 renamed from drivers/ide/ide-io.c 2005-01-04 09:31:45.000000000 -0500 +0 -1
@@ -1205,21 +1205,20 @@ HWGROUP(drive)->rq = NULL; rq->errors = 0; if (!rq->bio) goto out; rq->sector = rq->bio->bi_sector; rq->current_nr_sectors = bio_iovec(rq->bio)->bv_len >> 9; rq->hard_cur_sectors = rq->current_nr_sectors; - rq->buffer = NULL;
out:
return ret; }
/**
* ide_timer_expiry - handle lack of an IDE interrupt
* @data: timer callback magic (hwgroup)
*
* An IDE command has timed out before the expected drive return
* occurred. At this point we attempt to clean up the current
Quoted 55 lines · Prarit Bhargava
Hello,
I have found an IDE bug in the IDE DMA timeout function,
ide-io.c:ide_dma_timeout_retryerroneously sets thefirst_rq->buffer= NULL.
ide_dma_timeout_retrywill be called whenever a command is issued, times out,
and the drive is waiting for DMA. The function,ide_dma_timeout_retry, un-busies the hardware group and attempts to clean up the current request. As part of this cleanup the current failedfirst_rq->bufferis set to NULL.However, as part of this retry process
first_rqis retried up to 3 times in PIO mode (with DMA off).During the retry,
ide-cd.c:cdrom_start_readis called, which in turn calls,restore_request.restore_requestreferencesfirst_rq->buffer(which is
NULL) in order to calculatehard_cur_sectors,hard_nr_sectors, etc.ie) All of these values will be bogus because of the
first_rq->buffer= NULL.This request will fail and the IDE core will enter error handling. IDE core generates a new request,
sense_rq, in order to request sense. Attached this request is a back pointer to the originalfirst_rqrequest.ie) sense_rq->buffer = first_rqEventually ide-cd.c:cdrom_end_request is called on
sense_rq, and then ide-io.c:ide_end_dequeued_request is called onfirst_rq. Note thatide_end_dequeued_requestis called with the bogus values fromfirst_rq.The return value essentially depends on the return value of ll_rw_blk.c:__end_that_request_first. The arguements to this function include
nr_sectors, which as noted above, is bogus.This leads to a return of 1 from ll_rw_blk.c:__end_that_request_first which eventually leads to an erroneous call to
BUG()in ide-cd.c:cdrom_end_request.I have forced this issue to occur by modifying code to effectively DMA timeout on CDROM accesses on i686 and ia64 platforms. I hit the bug 100% of the time.
It appears that the modification should be to rid the
ide-io.ccode of therq->buffer= NULL call.Patch is based off of latest BK linux-2.5 as of 2005-01-04 09:00.
drivers/ide/ide-io.c 2005-01-04 09:32:23.000000000 -0500 renamed from drivers/ide/ide-io.c 2005-01-04 09:31:45.000000000 -0500 +0
-0@@ -1205,21 +1205,20 @@ HWGROUP(drive)->rq = NULL;
drivers/ide/ide-io.c 2005-01-04 09:32:23.000000000 -0500 line 1215
Quoted 10 lines
drivers/ide/ide-io.c 2005-01-04 09:32:23.000000000 -0500 renamed from drivers/ide/ide-io.c 2005-01-04 09:31:45.000000000 -0500 +0
-1rq->errors = 0; if (!rq->bio) goto out; rq->sector = rq->bio->bi_sector; rq->current_nr_sectors = bio_iovec(rq->bio)->bv_len >> 9; rq->hard_cur_sectors = rq->current_nr_sectors; - rq->buffer = NULL;
Probably safer to do
rq->buffer = bio_data(rq->bio);
Signature · 1 line
Jens Axboe
Thanks Jens,
I can your issue of correcting the position of the buffer. I've tested with this patch and do not hit the BUG().
P.
drivers/ide/ide-io.c 2005-01-04 15:45:27.000000000 -0500 renamed from drivers/ide/ide-io.c 2005-01-04 15:45:17.000000000 -0500 +1 -1
@@ -1205,21 +1205,21 @@ HWGROUP(drive)->rq = NULL; rq->errors = 0; if (!rq->bio) goto out; rq->sector = rq->bio->bi_sector; rq->current_nr_sectors = bio_iovec(rq->bio)->bv_len >> 9; rq->hard_cur_sectors = rq->current_nr_sectors; - rq->buffer = NULL; + rq->buffer = bio_data(rq->bio);
out:
return ret; }
/**
* ide_timer_expiry - handle lack of an IDE interrupt
* @data: timer callback magic (hwgroup)
*
* An IDE command has timed out before the expected drive return
* occurred. At this point we attempt to clean up the current
Quoted 72 lines · Jens Axboe
Quoted 52 lines · Prarit Bhargava
Hello,
I have found an IDE bug in the IDE DMA timeout function,
ide-io.c:ide_dma_timeout_retryerroneously sets thefirst_rq->buffer= NULL.
ide_dma_timeout_retrywill be called whenever a command is issued, times out, and the drive is waiting for DMA. The function,ide_dma_timeout_retry, un-busies the hardware group and attempts to clean up the current request. As part of this cleanup the current failedfirst_rq->bufferis set to NULL.However, as part of this retry process
first_rqis retried up to 3 times in PIO mode (with DMA off).During the retry,
ide-cd.c:cdrom_start_readis called, which in turn calls,restore_request.restore_requestreferencesfirst_rq->buffer(which is NULL) in order to calculatehard_cur_sectors,hard_nr_sectors, etc.ie) All of these values will be bogus because of the
first_rq->buffer= NULL.This request will fail and the IDE core will enter error handling. IDE core generates a new request,
sense_rq, in order to request sense. Attached this request is a back pointer to the originalfirst_rqrequest.ie) sense_rq->buffer = first_rqEventually ide-cd.c:cdrom_end_request is called on
sense_rq, and then ide-io.c:ide_end_dequeued_request is called onfirst_rq. Note thatide_end_dequeued_requestis called with the bogus values fromfirst_rq.The return value essentially depends on the return value of ll_rw_blk.c:__end_that_request_first. The arguements to this function include
nr_sectors, which as noted above, is bogus.This leads to a return of 1 from ll_rw_blk.c:__end_that_request_first which eventually leads to an erroneous call to
BUG()in ide-cd.c:cdrom_end_request.I have forced this issue to occur by modifying code to effectively DMA timeout on CDROM accesses on i686 and ia64 platforms. I hit the bug 100% of the time.
It appears that the modification should be to rid the
ide-io.ccode of therq->buffer= NULL call.Patch is based off of latest BK linux-2.5 as of 2005-01-04 09:00.
drivers/ide/ide-io.c 2005-01-04 09:32:23.000000000 -0500 renamed from drivers/ide/ide-io.c 2005-01-04 09:31:45.000000000 -0500 +0
-0@@ -1205,21 +1205,20 @@ HWGROUP(drive)->rq = NULL; rq->errors = 0;drivers/ide/ide-io.c 2005-01-04 09:32:23.000000000 -0500 line 1216
Quoted 10 lines
drivers/ide/ide-io.c 2005-01-04 09:32:23.000000000 -0500 renamed from drivers/ide/ide-io.c 2005-01-04 09:31:45.000000000 -0500 +0
-1if (!rq->bio) goto out; rq->sector = rq->bio->bi_sector; rq->current_nr_sectors = bio_iovec(rq->bio)->bv_len >> 9; rq->hard_cur_sectors = rq->current_nr_sectors; - rq->buffer = NULL;Probably safer to do
rq->buffer = bio_data(rq->bio);