2004-03-23 00:40:57

by Samuel Rydh

[permalink] [raw]
Subject: ide-cd bug (MODE_SENSE/CDROM_SEND_PACKET)


If a MODE_SENSE(6) command is sent to an IDE cd using the CDROM_SEND_PACKET
ioctl, then the kernel freezes solidly. To reproduce this, one can take the
SCSI cmd [1a 08 31 00 10 00] and a 16 byte data buffer.

After some bug hunting, I found out that the following is what happens:

- ide-cd recognizes that MODE_SENSE(6) isn't supported and tries
to abort the request from ide_cdrom_prep_pc by returning BLKPREP_KILL.

- in elv_next_request(), the kill request is handled by
the following code:

while (end_that_request_first(rq, 0, rq->nr_sectors))
;
end_that_request_last(rq);

The while loop never exits. The end_that_request_first() doesn't do anything
since rq->nr_sectors is 0; it just returns "not-done" after handling those 0
bytes (rq->bio->bi_size is 16).

I'm not quite sure how to fix this properly. For one thing, the data buffer
associated with the MODE_SENSE command is not sector sized in the first
place...

This is with a recent 2.6.5-rc1 kernel built from the BK tree.


/Samuel


2004-03-23 10:48:54

by Jens Axboe

[permalink] [raw]
Subject: Re: ide-cd bug (MODE_SENSE/CDROM_SEND_PACKET)

On Tue, Mar 23 2004, Samuel Rydh wrote:
>
> If a MODE_SENSE(6) command is sent to an IDE cd using the CDROM_SEND_PACKET
> ioctl, then the kernel freezes solidly. To reproduce this, one can take the
> SCSI cmd [1a 08 31 00 10 00] and a 16 byte data buffer.
>
> After some bug hunting, I found out that the following is what happens:
>
> - ide-cd recognizes that MODE_SENSE(6) isn't supported and tries
> to abort the request from ide_cdrom_prep_pc by returning BLKPREP_KILL.
>
> - in elv_next_request(), the kill request is handled by
> the following code:
>
> while (end_that_request_first(rq, 0, rq->nr_sectors))
> ;
> end_that_request_last(rq);
>
> The while loop never exits. The end_that_request_first() doesn't do anything
> since rq->nr_sectors is 0; it just returns "not-done" after handling those 0
> bytes (rq->bio->bi_size is 16).
>
> I'm not quite sure how to fix this properly. For one thing, the data buffer
> associated with the MODE_SENSE command is not sector sized in the first
> place...
>
> This is with a recent 2.6.5-rc1 kernel built from the BK tree.

Could you check if this works for you?

===== drivers/block/elevator.c 1.53 vs edited =====
--- 1.53/drivers/block/elevator.c Mon Jan 19 07:38:36 2004
+++ edited/drivers/block/elevator.c Tue Mar 23 11:47:53 2004
@@ -210,10 +210,14 @@
rq = NULL;
break;
} else if (ret == BLKPREP_KILL) {
+ int nr_bytes = rq->hard_nr_sectors << 9;
+
+ if (!nr_bytes)
+ nr_bytes = rq->data_len;
+
blkdev_dequeue_request(rq);
rq->flags |= REQ_QUIET;
- while (end_that_request_first(rq, 0, rq->nr_sectors))
- ;
+ end_that_request_chunk(rq, 0, nr_bytes);
end_that_request_last(rq);
} else {
printk("%s: bad return=%d\n", __FUNCTION__, ret);

--
Jens Axboe

2004-03-23 11:46:54

by Samuel Rydh

[permalink] [raw]
Subject: Re: ide-cd bug (MODE_SENSE/CDROM_SEND_PACKET)

On Tue, Mar 23, 2004 at 11:48:47AM +0100, Jens Axboe wrote:
> Could you check if this works for you?
>
> ===== drivers/block/elevator.c 1.53 vs edited =====
> --- 1.53/drivers/block/elevator.c Mon Jan 19 07:38:36 2004
> +++ edited/drivers/block/elevator.c Tue Mar 23 11:47:53 2004
> @@ -210,10 +210,14 @@
> rq = NULL;
> break;
> } else if (ret == BLKPREP_KILL) {
> + int nr_bytes = rq->hard_nr_sectors << 9;
> +
> + if (!nr_bytes)
> + nr_bytes = rq->data_len;
> +
> blkdev_dequeue_request(rq);
> rq->flags |= REQ_QUIET;
> - while (end_that_request_first(rq, 0, rq->nr_sectors))
> - ;
> + end_that_request_chunk(rq, 0, nr_bytes);
> end_that_request_last(rq);
> } else {
> printk("%s: bad return=%d\n", __FUNCTION__, ret);
>

Yes, this fixes the problem.

/Samuel