Commit 51490c89 authored by Pete Zaitcev's avatar Pete Zaitcev Committed by James Bottomley

[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: default avatarJames Bottomley <James.Bottomley@SteelEye.com>
parent 8224bfa8
...@@ -199,15 +199,7 @@ int sr_media_change(struct cdrom_device_info *cdi, int slot) ...@@ -199,15 +199,7 @@ int sr_media_change(struct cdrom_device_info *cdi, int slot)
/* check multisession offset etc */ /* check multisession offset etc */
sr_cd_check(cdi); sr_cd_check(cdi);
/* get_sectorsize(cd);
* 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;
} }
return retval; return retval;
} }
...@@ -538,13 +530,6 @@ static int sr_open(struct cdrom_device_info *cdi, int purpose) ...@@ -538,13 +530,6 @@ static int sr_open(struct cdrom_device_info *cdi, int purpose)
if (!scsi_block_when_processing_errors(sdev)) if (!scsi_block_when_processing_errors(sdev))
goto error_out; 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; return 0;
error_out: error_out:
...@@ -604,7 +589,6 @@ static int sr_probe(struct device *dev) ...@@ -604,7 +589,6 @@ static int sr_probe(struct device *dev)
cd->driver = &sr_template; cd->driver = &sr_template;
cd->disk = disk; cd->disk = disk;
cd->capacity = 0x1fffff; cd->capacity = 0x1fffff;
cd->needs_sector_size = 1;
cd->device->changed = 1; /* force recheck CD type */ cd->device->changed = 1; /* force recheck CD type */
cd->use = 1; cd->use = 1;
cd->readcd_known = 0; cd->readcd_known = 0;
...@@ -694,7 +678,6 @@ static void get_sectorsize(struct scsi_cd *cd) ...@@ -694,7 +678,6 @@ static void get_sectorsize(struct scsi_cd *cd)
if (the_result) { if (the_result) {
cd->capacity = 0x1fffff; cd->capacity = 0x1fffff;
sector_size = 2048; /* A guess, just in case */ sector_size = 2048; /* A guess, just in case */
cd->needs_sector_size = 1;
} else { } else {
#if 0 #if 0
if (cdrom_get_last_written(&cd->cdi, if (cdrom_get_last_written(&cd->cdi,
...@@ -727,7 +710,6 @@ static void get_sectorsize(struct scsi_cd *cd) ...@@ -727,7 +710,6 @@ static void get_sectorsize(struct scsi_cd *cd)
printk("%s: unsupported sector size %d.\n", printk("%s: unsupported sector size %d.\n",
cd->cdi.name, sector_size); cd->cdi.name, sector_size);
cd->capacity = 0; 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) ...@@ -736,7 +718,6 @@ static void get_sectorsize(struct scsi_cd *cd)
* Add this so that we have the ability to correctly gauge * Add this so that we have the ability to correctly gauge
* what the device is capable of. * 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 @@ static void get_sectorsize(struct scsi_cd *cd) ...@@ -748,8 +729,7 @@ static void get_sectorsize(struct scsi_cd *cd)
Enomem: Enomem:
cd->capacity = 0x1fffff; cd->capacity = 0x1fffff;
sector_size = 2048; /* A guess, just in case */ cd->device->sector_size = 2048; /* A guess, just in case */
cd->needs_sector_size = 1;
if (SRpnt) if (SRpnt)
scsi_release_request(SRpnt); scsi_release_request(SRpnt);
goto out; goto out;
......
...@@ -33,7 +33,6 @@ typedef struct scsi_cd { ...@@ -33,7 +33,6 @@ typedef struct scsi_cd {
struct scsi_device *device; struct scsi_device *device;
unsigned int vendor; /* vendor code, see sr_vendor.c */ unsigned int vendor; /* vendor code, see sr_vendor.c */
unsigned long ms_offset; /* for reading multisession-CD's */ 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 use:1; /* is this device still supportable */
unsigned xa_flag:1; /* CD has XA sectors ? */ unsigned xa_flag:1; /* CD has XA sectors ? */
unsigned readcd_known:1; /* drive supports READ_CD (0xbe) */ unsigned readcd_known:1; /* drive supports READ_CD (0xbe) */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment