From: Samuel Thibault 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 --- 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-08-04 20:39:25.611792984 -0700 +++ 25-akpm/drivers/cdrom/cdrom.c 2004-08-04 20:39:25.618791920 -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) @@ -2837,7 +2837,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; @@ -2850,14 +2850,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 */ @@ -2865,7 +2869,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); @@ -2879,14 +2883,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 @@ -2897,27 +2905,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 */ @@ -2930,11 +2944,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; @@ -2947,32 +2962,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))) { _