Re: CDROM bug in 2.3.x

Jens Axboe (axboe@image.dk)
Thu, 2 Sep 1999 18:48:30 +0200


--fUYQa+Pmc3FrFX/N
Content-Type: text/plain; charset=us-ascii

On Wed, Sep 01 1999, David S. Miller wrote:
> This one has been there for a while, and I finally sat
> down just now to track it down.
>
> What I see is that the generic cdrom driver is passing
> down packet commands with a buffer length which is
> negative. I see that some of the cgc command building
> routines are setting negative buffer lengths on purpose.
>
> This is illegal and is confusing SCSI drivers quite
> badly, because the scsi command will have
> SCpnt->request_bufflen < 0 and this will be given to
> the controller for the DMA request.
>
> I don't know what the intentions were here, but this
> does need to be fixed somehow so I leave it to Jens
> to figure out the correct fix.

Alright, here's the fix. ide-cd.c had some really odd
code, where it expected transfers going to the drive
to have negative buffer lengths...

-- 
*  Jens Axboe <axboe@image.dk>
*  Linux CD-ROM Maintainer
*  http://www.kernel.dk

--fUYQa+Pmc3FrFX/N Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="sep2.diff"

diff -ur --exclude-from /home/axboe/cdrom/exclude-from /tmp/linux-2.3.16/drivers/block/ide-cd.c linux/drivers/block/ide-cd.c --- /tmp/linux-2.3.16/drivers/block/ide-cd.c Wed Sep 1 11:29:53 1999 +++ linux/drivers/block/ide-cd.c Thu Sep 2 17:20:10 1999 @@ -1247,20 +1247,10 @@ /* Figure out how much data to transfer. */ thislen = pc->buflen; - if (thislen < 0) thislen = -thislen; if (thislen > len) thislen = len; /* The drive wants to be written to. */ if ((ireason & 3) == 0) { - /* Check that we want to write. */ - if (pc->buflen > 0) { - printk ("%s: cdrom_pc_intr: Drive wants " - "to transfer data the wrong way!\n", - drive->name); - pc->stat = 1; - thislen = 0; - } - /* Transfer the data. */ atapi_output_bytes (drive, pc->buffer, thislen); @@ -1274,19 +1264,11 @@ /* Keep count of how much data we've moved. */ pc->buffer += thislen; - pc->buflen += thislen; + pc->buflen -= thislen; } /* Same drill for reading. */ else if ((ireason & 3) == 2) { - /* Check that we want to read. */ - if (pc->buflen < 0) { - printk ("%s: cdrom_pc_intr: Drive wants to " - "transfer data the wrong way!\n", - drive->name); - pc->stat = 1; - thislen = 0; - } /* Transfer the data. */ atapi_input_bytes (drive, pc->buffer, thislen); @@ -1794,10 +1776,13 @@ toc->xa_flag = (ms_tmp.hdr.first_track != ms_tmp.hdr.last_track); /* Now try to get the total cdrom capacity. */ +#if 0 stat = cdrom_get_last_written(MKDEV(HWIF(drive)->major, drive->select.b.unit << PARTN_BITS), (long *)&toc->capacity); - if (stat) stat = cdrom_read_capacity (drive, &toc->capacity, reqbuf); + if (stat) +#endif + stat = cdrom_read_capacity (drive, &toc->capacity, reqbuf); if (stat) toc->capacity = 0x1fffff; HWIF(drive)->gd->sizes[drive->select.b.unit << PARTN_BITS] @@ -2048,7 +2033,6 @@ return cgc->stat; } - static int ide_cdrom_dev_ioctl (struct cdrom_device_info *cdi, unsigned int cmd, unsigned long arg) @@ -2977,11 +2961,3 @@ MOD_DEC_USE_COUNT; return 0; } - - -/*==========================================================================*/ -/* - * Local variables: - * c-basic-offset: 8 - * End: - */ diff -ur --exclude-from /home/axboe/cdrom/exclude-from /tmp/linux-2.3.16/drivers/cdrom/cdrom.c linux/drivers/cdrom/cdrom.c --- /tmp/linux-2.3.16/drivers/cdrom/cdrom.c Wed Sep 1 11:29:54 1999 +++ linux/drivers/cdrom/cdrom.c Thu Sep 2 17:21:23 1999 @@ -473,7 +473,8 @@ /* give people a warning shot, now that CDO_CHECK_TYPE is the default case! */ cdinfo(CD_OPEN, "bummer. wrong media type.\n"); - cdinfo(CD_WARNING, "pid %d is buggy!\n", (unsigned int)current->pid); + cdinfo(CD_WARNING, "pid %d must open device O_NONBLOCK!\n", + (unsigned int)current->pid); ret=-EMEDIUMTYPE; goto clean_up_and_return; } @@ -1080,13 +1081,8 @@ cgc->cmd[0] = GPCMD_MODE_SELECT_10; cgc->cmd[1] = 0x10; /* PF */ - - /* generic_packet() wants the length as seen from the drive, i.e. - it will transfer data _to_ us. The CD-ROM wants the absolute - value, however. */ - cgc->cmd[7] = (-cgc->buflen) >> 8; - cgc->cmd[8] = (-cgc->buflen) & 0xff; - + cgc->cmd[7] = cgc->buflen >> 8; + cgc->cmd[8] = cgc->buflen & 0xff; return cdo->generic_packet(cdi, cgc); } @@ -1734,7 +1730,6 @@ memset(buffer, 0, 3); /* set volume */ - cgc.buflen = -cgc.buflen; cgc.buffer = buffer; return cdrom_mode_select(cdi, &cgc); } @@ -2234,12 +2229,3 @@ } #endif /* endif MODULE */ - - - -/* - * Local variables: - * comment-column: 40 - * compile-command: "gcc -D__KERNEL__ -I/usr/src/linux/include -Wall -Wstrict-prototypes -O2 -fomit-frame-pointer -pipe -fno-strength-reduce -m486 -DCPU=486 -DMODULE -DMODVERSIONS -include /usr/src/linux/include/linux/modversions.h -c -o cdrom.o cdrom.c" - * End: - */ diff -ur --exclude-from /home/axboe/cdrom/exclude-from /tmp/linux-2.3.16/drivers/scsi/sr.c linux/drivers/scsi/sr.c --- /tmp/linux-2.3.16/drivers/scsi/sr.c Wed Sep 1 11:29:55 1999 +++ linux/drivers/scsi/sr.c Wed Sep 1 11:39:34 1999 @@ -928,13 +928,14 @@ scsi_CDs[i].sector_size = 2048; /* A guess, just in case */ scsi_CDs[i].needs_sector_size = 1; } else { +#if 0 if (cdrom_get_last_written(MKDEV(MAJOR_NR, i), - (long*)&scsi_CDs[i].capacity)) { + (long*)&scsi_CDs[i].capacity)) +#endif scsi_CDs[i].capacity = 1 + ((buffer[0] << 24) | (buffer[1] << 16) | (buffer[2] << 8) | buffer[3]); - } scsi_CDs[i].sector_size = (buffer[4] << 24) | (buffer[5] << 16) | (buffer[6] << 8) | buffer[7]; switch (scsi_CDs[i].sector_size) {

--fUYQa+Pmc3FrFX/N--

- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/