diff options
author | Pete Zaitcev <zaitcev@redhat.com> | 2005-07-05 21:18:08 -0400 |
---|---|---|
committer | James Bottomley <jejb@titanic.(none)> | 2005-08-28 12:14:12 -0400 |
commit | 51490c89f95b8581782e9baa855da166441852be (patch) | |
tree | 3be27d5e30c114d5d69fe21a4a079a64f2016354 | |
parent | 8224bfa84d510630b40ea460b2bb380c91acb8ae (diff) |
[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>
-rw-r--r-- | drivers/scsi/sr.c | 24 | ||||
-rw-r--r-- | drivers/scsi/sr.h | 1 |
2 files changed, 2 insertions, 23 deletions
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c index 2f259f249522..f63d8c6c2a33 100644 --- 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) | |||
199 | /* check multisession offset etc */ | 199 | /* check multisession offset etc */ |
200 | sr_cd_check(cdi); | 200 | sr_cd_check(cdi); |
201 | 201 | ||
202 | /* | 202 | get_sectorsize(cd); |
203 | * If the disk changed, the capacity will now be different, | ||
204 | * so we force a re-read of this information | ||
205 | * Force 2048 for the sector size so that filesystems won't | ||
206 | * be trying to use something that is too small if the disc | ||
207 | * has changed. | ||
208 | */ | ||
209 | cd->needs_sector_size = 1; | ||
210 | cd->device->sector_size = 2048; | ||
211 | } | 203 | } |
212 | return retval; | 204 | return retval; |
213 | } | 205 | } |
@@ -538,13 +530,6 @@ static int sr_open(struct cdrom_device_info *cdi, int purpose) | |||
538 | if (!scsi_block_when_processing_errors(sdev)) | 530 | if (!scsi_block_when_processing_errors(sdev)) |
539 | goto error_out; | 531 | goto error_out; |
540 | 532 | ||
541 | /* | ||
542 | * If this device did not have media in the drive at boot time, then | ||
543 | * we would have been unable to get the sector size. Check to see if | ||
544 | * this is the case, and try again. | ||
545 | */ | ||
546 | if (cd->needs_sector_size) | ||
547 | get_sectorsize(cd); | ||
548 | return 0; | 533 | return 0; |
549 | 534 | ||
550 | error_out: | 535 | error_out: |
@@ -604,7 +589,6 @@ static int sr_probe(struct device *dev) | |||
604 | cd->driver = &sr_template; | 589 | cd->driver = &sr_template; |
605 | cd->disk = disk; | 590 | cd->disk = disk; |
606 | cd->capacity = 0x1fffff; | 591 | cd->capacity = 0x1fffff; |
607 | cd->needs_sector_size = 1; | ||
608 | cd->device->changed = 1; /* force recheck CD type */ | 592 | cd->device->changed = 1; /* force recheck CD type */ |
609 | cd->use = 1; | 593 | cd->use = 1; |
610 | cd->readcd_known = 0; | 594 | cd->readcd_known = 0; |
@@ -694,7 +678,6 @@ static void get_sectorsize(struct scsi_cd *cd) | |||
694 | if (the_result) { | 678 | if (the_result) { |
695 | cd->capacity = 0x1fffff; | 679 | cd->capacity = 0x1fffff; |
696 | sector_size = 2048; /* A guess, just in case */ | 680 | sector_size = 2048; /* A guess, just in case */ |
697 | cd->needs_sector_size = 1; | ||
698 | } else { | 681 | } else { |
699 | #if 0 | 682 | #if 0 |
700 | if (cdrom_get_last_written(&cd->cdi, | 683 | if (cdrom_get_last_written(&cd->cdi, |
@@ -727,7 +710,6 @@ static void get_sectorsize(struct scsi_cd *cd) | |||
727 | printk("%s: unsupported sector size %d.\n", | 710 | printk("%s: unsupported sector size %d.\n", |
728 | cd->cdi.name, sector_size); | 711 | cd->cdi.name, sector_size); |
729 | cd->capacity = 0; | 712 | cd->capacity = 0; |
730 | cd->needs_sector_size = 1; | ||
731 | } | 713 | } |
732 | 714 | ||
733 | cd->device->sector_size = sector_size; | 715 | cd->device->sector_size = sector_size; |
@@ -736,7 +718,6 @@ static void get_sectorsize(struct scsi_cd *cd) | |||
736 | * Add this so that we have the ability to correctly gauge | 718 | * Add this so that we have the ability to correctly gauge |
737 | * what the device is capable of. | 719 | * what the device is capable of. |
738 | */ | 720 | */ |
739 | cd->needs_sector_size = 0; | ||
740 | set_capacity(cd->disk, cd->capacity); | 721 | set_capacity(cd->disk, cd->capacity); |
741 | } | 722 | } |
742 | 723 | ||
@@ -748,8 +729,7 @@ out: | |||
748 | 729 | ||
749 | Enomem: | 730 | Enomem: |
750 | cd->capacity = 0x1fffff; | 731 | cd->capacity = 0x1fffff; |
751 | sector_size = 2048; /* A guess, just in case */ | 732 | cd->device->sector_size = 2048; /* A guess, just in case */ |
752 | cd->needs_sector_size = 1; | ||
753 | if (SRpnt) | 733 | if (SRpnt) |
754 | scsi_release_request(SRpnt); | 734 | scsi_release_request(SRpnt); |
755 | goto out; | 735 | goto out; |
diff --git a/drivers/scsi/sr.h b/drivers/scsi/sr.h index 0b3178007203..d2bcd99c272f 100644 --- a/drivers/scsi/sr.h +++ b/drivers/scsi/sr.h | |||
@@ -33,7 +33,6 @@ typedef struct scsi_cd { | |||
33 | struct scsi_device *device; | 33 | struct scsi_device *device; |
34 | unsigned int vendor; /* vendor code, see sr_vendor.c */ | 34 | unsigned int vendor; /* vendor code, see sr_vendor.c */ |
35 | unsigned long ms_offset; /* for reading multisession-CD's */ | 35 | unsigned long ms_offset; /* for reading multisession-CD's */ |
36 | unsigned needs_sector_size:1; /* needs to get sector size */ | ||
37 | unsigned use:1; /* is this device still supportable */ | 36 | unsigned use:1; /* is this device still supportable */ |
38 | unsigned xa_flag:1; /* CD has XA sectors ? */ | 37 | unsigned xa_flag:1; /* CD has XA sectors ? */ |
39 | unsigned readcd_known:1; /* drive supports READ_CD (0xbe) */ | 38 | unsigned readcd_known:1; /* drive supports READ_CD (0xbe) */ |