From: Samuel Thibault <samuel.thibault@ens-lyon.org>
There's something wrong in cdrom.c: cdrom_get_last_written() for instance
calls cdrom_get_disc_info() and cdrom_get_track_info() to get information
about tracks, but these functions don't ensure that all the
track_information or disc_information structure is filled:
/* (buflen was first set to 8 to get track_information_length field) */
if ((ret = cdo->generic_packet(cdi, &cgc)))
return ret;
cgc.buflen = be16_to_cpu(ti->track_information_length) +
sizeof(ti->track_information_length);
if (cgc.buflen > sizeof(track_information))
cgc.buflen = sizeof(track_information);
cgc.cmd[8] = cgc.buflen;
return cdo->generic_packet(cdi, &cgc);
The second test ensures that at least we won't overflow the structure, but
nothing ensures that all the structure will be filled.
And indeed, we have a drive here that won't fill it all: the returned
track_information_length field will be *less than*
sizeof(track_information) - sizeof(ti->track_information_length), so that
cdrom_get_last_written() reads values that weren't filled in! As a result,
we are sometimes unable to read some parts of CDROMs, depending on the
uninitialized state of the structure...
Here is a patch that adds filling checks: cdrom_get_disc_info() and
cdrom_get_track_info() return the actual filled length, and it's up to the
caller to check that this is enough for him to get the values it wants.
Note: adding something like a
#define spanof(TYPE, MEMBER) ((size_t) ((&((TYPE *)0)->MEMBER)+1))
definition just near that of offsetof() in include/linux/stddef.h would
make it more pretty, but still it won't help for bitfields :/
Signed-off-by: Andrew Morton <akpm@osdl.org>
---
25-akpm/drivers/cdrom/cdrom.c | 82 +++++++++++++++++++++++++-----------------
1 files changed, 49 insertions(+), 33 deletions(-)
diff -puN drivers/cdrom/cdrom.c~cdrom-get_last_written-fix drivers/cdrom/cdrom.c
--- 25/drivers/cdrom/cdrom.c~cdrom-get_last_written-fix 2004-07-26 16:32:58.624497536 -0700
+++ 25-akpm/drivers/cdrom/cdrom.c 2004-07-26 16:32:58.639495256 -0700
@@ -607,7 +607,7 @@ static int cdrom_mrw_exit(struct cdrom_d
disc_information di;
int ret = 0;
- if (cdrom_get_disc_info(cdi, &di))
+ if (cdrom_get_disc_info(cdi, &di) < offsetof(typeof(di),disc_type))
return 1;
if (di.mrw_status == CDM_MRW_BGFORMAT_ACTIVE) {
@@ -716,7 +716,7 @@ static int cdrom_media_erasable(struct c
{
disc_information di;
- if (cdrom_get_disc_info(cdi, &di))
+ if (cdrom_get_disc_info(cdi, &di) < offsetof(typeof(di),n_first_track))
return -1;
return di.erasable;
@@ -752,7 +752,7 @@ static int cdrom_mrw_open_write(struct c
return 1;
}
- if (cdrom_get_disc_info(cdi, &di))
+ if (cdrom_get_disc_info(cdi, &di) < offsetof(typeof(di),disc_type))
return 1;
if (!di.erasable)
@@ -2832,7 +2832,7 @@ static int cdrom_get_track_info(struct c
{
struct cdrom_device_ops *cdo = cdi->ops;
struct packet_command cgc;
- int ret;
+ int ret, buflen;
init_cdrom_command(&cgc, ti, 8, CGC_DATA_READ);
cgc.cmd[0] = GPCMD_READ_TRACK_RZONE_INFO;
@@ -2845,14 +2845,18 @@ static int cdrom_get_track_info(struct c
if ((ret = cdo->generic_packet(cdi, &cgc)))
return ret;
- cgc.buflen = be16_to_cpu(ti->track_information_length) +
+ buflen = be16_to_cpu(ti->track_information_length) +
sizeof(ti->track_information_length);
- if (cgc.buflen > sizeof(track_information))
- cgc.buflen = sizeof(track_information);
+ if (buflen > sizeof(track_information))
+ buflen = sizeof(track_information);
- cgc.cmd[8] = cgc.buflen;
- return cdo->generic_packet(cdi, &cgc);
+ cgc.cmd[8] = cgc.buflen = buflen;
+ if ((ret = cdo->generic_packet(cdi, &cgc)))
+ return ret;
+
+ /* return actual fill size */
+ return buflen;
}
/* requires CD R/RW */
@@ -2860,7 +2864,7 @@ static int cdrom_get_disc_info(struct cd
{
struct cdrom_device_ops *cdo = cdi->ops;
struct packet_command cgc;
- int ret;
+ int ret, buflen;
/* set up command and get the disc info */
init_cdrom_command(&cgc, di, sizeof(*di), CGC_DATA_READ);
@@ -2874,14 +2878,18 @@ static int cdrom_get_disc_info(struct cd
/* not all drives have the same disc_info length, so requeue
* packet with the length the drive tells us it can supply
*/
- cgc.buflen = be16_to_cpu(di->disc_information_length) +
+ buflen = be16_to_cpu(di->disc_information_length) +
sizeof(di->disc_information_length);
- if (cgc.buflen > sizeof(disc_information))
- cgc.buflen = sizeof(disc_information);
+ if (buflen > sizeof(disc_information))
+ buflen = sizeof(disc_information);
- cgc.cmd[8] = cgc.buflen;
- return cdo->generic_packet(cdi, &cgc);
+ cgc.cmd[8] = cgc.buflen = buflen;
+ if ((ret = cdo->generic_packet(cdi, &cgc)))
+ return ret;
+
+ /* return actual fill size */
+ return buflen;
}
/* return the last written block on the CD-R media. this is for the udf
@@ -2892,27 +2900,33 @@ int cdrom_get_last_written(struct cdrom_
disc_information di;
track_information ti;
__u32 last_track;
- int ret = -1;
+ int ret = -1, ti_size;
if (!CDROM_CAN(CDC_GENERIC_PACKET))
goto use_toc;
- if ((ret = cdrom_get_disc_info(cdi, &di)))
+ if ((ret = cdrom_get_disc_info(cdi, &di))
+ < offsetof(typeof(di), last_track_msb)
+ + sizeof(di.last_track_msb))
goto use_toc;
last_track = (di.last_track_msb << 8) | di.last_track_lsb;
- if ((ret = cdrom_get_track_info(cdi, last_track, 1, &ti)))
+ ti_size = cdrom_get_track_info(cdi, last_track, 1, &ti);
+ if (ti_size < offsetof(typeof(ti), track_start))
goto use_toc;
/* if this track is blank, try the previous. */
if (ti.blank) {
last_track--;
- if ((ret = cdrom_get_track_info(cdi, last_track, 1, &ti)))
- goto use_toc;
+ ti_size = cdrom_get_track_info(cdi, last_track, 1, &ti);
}
+ if (ti_size < offsetof(typeof(ti), track_size) + sizeof(ti.track_size))
+ goto use_toc;
+
/* if last recorded field is valid, return it. */
- if (ti.lra_v) {
+ if (ti.lra_v && ti_size >= offsetof(typeof(ti), last_rec_address)
+ + sizeof(ti.last_rec_address)) {
*last_written = be32_to_cpu(ti.last_rec_address);
} else {
/* make it up instead */
@@ -2925,11 +2939,12 @@ int cdrom_get_last_written(struct cdrom_
/* this is where we end up if the drive either can't do a
GPCMD_READ_DISC_INFO or GPCMD_READ_TRACK_RZONE_INFO or if
- it fails. then we return the toc contents. */
+ it doesn't give enough information or fails. then we return
+ the toc contents. */
use_toc:
toc.cdte_format = CDROM_MSF;
toc.cdte_track = CDROM_LEADOUT;
- if (cdi->ops->audio_ioctl(cdi, CDROMREADTOCENTRY, &toc))
+ if ((ret = cdi->ops->audio_ioctl(cdi, CDROMREADTOCENTRY, &toc)))
return ret;
sanitize_format(&toc.cdte_addr, &toc.cdte_format, CDROM_LBA);
*last_written = toc.cdte_addr.lba;
@@ -2942,32 +2957,33 @@ static int cdrom_get_next_writable(struc
disc_information di;
track_information ti;
__u16 last_track;
- int ret = -1;
+ int ret = -1, ti_size;
if (!CDROM_CAN(CDC_GENERIC_PACKET))
goto use_last_written;
- if ((ret = cdrom_get_disc_info(cdi, &di)))
+ if ((ret = cdrom_get_disc_info(cdi, &di))
+ < offsetof(typeof(di), last_track_msb)
+ + sizeof(di.last_track_msb))
goto use_last_written;
last_track = (di.last_track_msb << 8) | di.last_track_lsb;
- if ((ret = cdrom_get_track_info(cdi, last_track, 1, &ti)))
+ ti_size = cdrom_get_track_info(cdi, last_track, 1, &ti);
+ if (ti_size < offsetof(typeof(ti), track_start))
goto use_last_written;
/* if this track is blank, try the previous. */
if (ti.blank) {
last_track--;
- if ((ret = cdrom_get_track_info(cdi, last_track, 1, &ti)))
- goto use_last_written;
+ ti_size = cdrom_get_track_info(cdi, last_track, 1, &ti);
}
/* if next recordable address field is valid, use it. */
- if (ti.nwa_v)
+ if (ti.nwa_v && ti_size >= offsetof(typeof(ti), next_writable)
+ + sizeof(ti.next_writable)) {
*next_writable = be32_to_cpu(ti.next_writable);
- else
- goto use_last_written;
-
- return 0;
+ return 0;
+ }
use_last_written:
if ((ret = cdrom_get_last_written(cdi, next_writable))) {
_