[SCSI] sr.c: Fix getting wrong size

Here's the problem. Try to do this on 2.6.12:
- Kill udev and HAL
- Insert a CD-ROM into a SCSI or USB CD-ROM drive
- Run dd if=/dev/scd0
- cat /sys/block/sr0/size
- Eject the CD, insert a different one
- Run dd if=/dev/scd0
This is likely to do "access beyond the end of device", if you let it
- cat /sys/block/sr0/size
This shows the size of a previous CD, even though dd was supposed
to revalidate the device.
- Run dd if=/dev/scd0
The second run of dd works correctly!

The bug was introduced in 2.5.31, when Al fixes the recursive opens
in partitioning. Before, the code worked like this:
- Block layer called cdrom_open directly
- cdrom_open called sr_open
- sr_open called check_disk_change
- check_disk_change called sr_media_change
- sr_media_change did cd->needs_disk_change=1
- before returning sr_open tested cd->needs_disk_change
  and called get_sector_size.

In 2.6.12, the check_disk_change is called from cdrom_open only. Thus:
- Block layer calls sr_bd_open
- sr_bd_open calls cdrom_open
- cdrom_open calls sr_open
- sr_open tests cd->needs_disk_change, which wasn't set yet; returns
- cdrom_open calls check_disk_change
- check_disk_change calls sr_media_change
- sr_media_change does cd->needs_disk_change=1, but nobody cares

Acked by: Alexander Viro <aviro@redhat.com>
Signed-off-by: James Bottomley <James.Bottomley@SteelEye.com>

diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 2f259f2495229b54a1572492bfe3cc818d0dfbf7..f63d8c6c2a3314303a95544b14cc1d2c109a903a 100644 (file)
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -199,15 +199,7 @@ int sr_media_change(struct cdrom_device_info *cdi, int slot)
                /* check multisession offset etc */
                sr_cd_check(cdi);
 
                /* check multisession offset etc */
                sr_cd_check(cdi);
 

-               /* 
-                * If the disk changed, the capacity will now be different,
-                * so we force a re-read of this information 
-                * Force 2048 for the sector size so that filesystems won't
-                * be trying to use something that is too small if the disc
-                * has changed.
-                */
-               cd->needs_sector_size = 1;
-               cd->device->sector_size = 2048;
+               get_sectorsize(cd);

        }
        return retval;
 }
        }
        return retval;
 }


@@ -538,13 +530,6 @@ static int sr_open(struct cdrom_device_info *cdi, int purpose)
        if (!scsi_block_when_processing_errors(sdev))
                goto error_out;
 
        if (!scsi_block_when_processing_errors(sdev))
                goto error_out;
 

-       /*
-        * If this device did not have media in the drive at boot time, then
-        * we would have been unable to get the sector size.  Check to see if
-        * this is the case, and try again.
-        */
-       if (cd->needs_sector_size)
-               get_sectorsize(cd);

        return 0;
 
 error_out:
        return 0;
 
 error_out:


@@ -604,7 +589,6 @@ static int sr_probe(struct device *dev)
        cd->driver = &sr_template;
        cd->disk = disk;
        cd->capacity = 0x1fffff;
        cd->driver = &sr_template;
        cd->disk = disk;
        cd->capacity = 0x1fffff;

-       cd->needs_sector_size = 1;

        cd->device->changed = 1;        /* force recheck CD type */
        cd->use = 1;
        cd->readcd_known = 0;
        cd->device->changed = 1;        /* force recheck CD type */
        cd->use = 1;
        cd->readcd_known = 0;


@@ -694,7 +678,6 @@ static void get_sectorsize(struct scsi_cd *cd)
        if (the_result) {
                cd->capacity = 0x1fffff;
                sector_size = 2048;     /* A guess, just in case */
        if (the_result) {
                cd->capacity = 0x1fffff;
                sector_size = 2048;     /* A guess, just in case */

-               cd->needs_sector_size = 1;

        } else {
 #if 0
                if (cdrom_get_last_written(&cd->cdi,
        } else {
 #if 0
                if (cdrom_get_last_written(&cd->cdi,


@@ -727,7 +710,6 @@ static void get_sectorsize(struct scsi_cd *cd)
                        printk("%s: unsupported sector size %d.\n",
                               cd->cdi.name, sector_size);
                        cd->capacity = 0;
                        printk("%s: unsupported sector size %d.\n",
                               cd->cdi.name, sector_size);
                        cd->capacity = 0;

-                       cd->needs_sector_size = 1;

                }
 
                cd->device->sector_size = sector_size;
                }
 
                cd->device->sector_size = sector_size;


@@ -736,7 +718,6 @@ static void get_sectorsize(struct scsi_cd *cd)
                 * Add this so that we have the ability to correctly gauge
                 * what the device is capable of.
                 */
                 * Add this so that we have the ability to correctly gauge
                 * what the device is capable of.
                 */

-               cd->needs_sector_size = 0;

                set_capacity(cd->disk, cd->capacity);
        }
 
                set_capacity(cd->disk, cd->capacity);
        }
 


@@ -748,8 +729,7 @@ out:
 
 Enomem:
        cd->capacity = 0x1fffff;
 
 Enomem:
        cd->capacity = 0x1fffff;

-       sector_size = 2048;     /* A guess, just in case */
-       cd->needs_sector_size = 1;
+       cd->device->sector_size = 2048; /* A guess, just in case */

        if (SRpnt)
                scsi_release_request(SRpnt);
        goto out;
        if (SRpnt)
                scsi_release_request(SRpnt);
        goto out;

diff --git a/drivers/scsi/sr.h b/drivers/scsi/sr.h
index 0b317800720380b9029d3863e9a993a47757a0cb..d2bcd99c272f430970bb135264d13fa81c4e8ba3 100644 (file)
--- a/drivers/scsi/sr.h
+++ b/drivers/scsi/sr.h
@@ -33,7 +33,6 @@ typedef struct scsi_cd {
        struct scsi_device *device;
        unsigned int vendor;    /* vendor code, see sr_vendor.c         */
        unsigned long ms_offset;        /* for reading multisession-CD's        */
        struct scsi_device *device;
        unsigned int vendor;    /* vendor code, see sr_vendor.c         */
        unsigned long ms_offset;        /* for reading multisession-CD's        */

-       unsigned needs_sector_size:1;   /* needs to get sector size */

        unsigned use:1;         /* is this device still supportable     */
        unsigned xa_flag:1;     /* CD has XA sectors ? */
        unsigned readcd_known:1;        /* drive supports READ_CD (0xbe) */
        unsigned use:1;         /* is this device still supportable     */
        unsigned xa_flag:1;     /* CD has XA sectors ? */
        unsigned readcd_known:1;        /* drive supports READ_CD (0xbe) */