[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Libcdio-devel] libcdio freeze for 0.80 next weekend
|
From: |
R. Bernstein |
|
Subject: |
Re: [Libcdio-devel] libcdio freeze for 0.80 next weekend |
|
Date: |
Mon, 10 Mar 2008 22:08:15 -0400 |
Thanks for the patch. Here are my thoughts. I welcome other thoughts
and comments.
The main motivation for this release is to clear out bug fixes and
possible buffer overflows of the kind security people get excited
about. (In practice though I think the impact pretty small, but
I'm no security expert.)
(That said, it is however true that there have been some minor
feature enhancements like outputing cd-paranoia status information to
file.)
It was also proposed to convert the libcdio license to GPL v3, but
that was postponed because it was felt that it unfair to bundle bug
and security fixes with a license change.
Adding get_track_lba is an API change; also possibly an ABI change
since the cdio_funcs_t structure now grows to add one more function
(internal) function pointer.
At a minimum, we'll have to change libcdio_la_CURRENT, REVISION and
AGE. Nicholas Boullis is more of an expert on this here and so I hope
he will correct me here. The sense I get though is that because
libcdio is, well, a *library*, these kinds of changes require other
packages to get redone. So again it might be best to do the securty
fix and follow with a relase to change the API (and ABI?).
And while on the topic of good things to add, probably a couple of
regression tests for the bin/cue and nrg image files would be nice.
As for when the release after next is, that's entirely up how folks
feel about this. The next release after 0.80 could be for example
coordinated with the next cued release.
Thanks again.
Robert William Fuller writes:
> R. Bernstein wrote:
> > In preparation for a release next Saturday March 15, I guess a feature
> > freeze is in effect for libcdio. (Not there was all that much activity
> > anyway.)
>
> Hmm. Please consider the following patch for inclusion in spite of the
> freeze. Unlike the prior patches, it has been carefully considered, and
> the next release of cued depends on it.
>
> Sorry about all the spam with the prior patch attempt. I had broken my
> finger working in the yard, and I was a bit out of sorts. I did not
> realize my finger was broken until I went to the emergency care and got
> some X-rays. Anyway, you won't see 5 more patches in quick succession!
>
> This patch adds pregap support for Nero images and bincue. It has been
> carefully tested. I did not add support for CDRDAO because I do not
> have any CDRDAO images. I plan to add it when I need it.
>
> Thank you!
>
> Rob
> diff -upr -x version.h -x 'libiso9660*pc' -x 'libcdio*pc'
> libcdio-0.80-clean/include/cdio/track.h libcdio-0.80/include/cdio/track.h
> --- libcdio-0.80-clean/include/cdio/track.h 2006-01-23 15:30:28.000000000
> -0500
> +++ libcdio-0.80/include/cdio/track.h 2008-03-10 16:15:32.000000000
> -0400
> @@ -196,6 +196,28 @@ extern "C" {
> @return the starting LSN or CDIO_INVALID_LSN on error.
> */
> lsn_t cdio_get_track_lsn(const CdIo_t *p_cdio, track_t i_track);
> +
> + /*!
> + Return the starting LBA for the pregap for track number
> + i_track in p_cdio. Track numbers usually start at something
> + greater than 0, usually 1.
> +
> + @param p_cdio object to get information from
> + @param i_track the track number we want the LBA for
> + @return the starting LBA or CDIO_INVALID_LBA on error.
> + */
> + lba_t cdio_get_track_pregap_lba(const CdIo_t *p_cdio, track_t i_track);
> +
> + /*!
> + Return the starting LSN for the pregap for track number
> + i_track in p_cdio. Track numbers usually start at something
> + greater than 0, usually 1.
> +
> + @param p_cdio object to get information from
> + @param i_track the track number we want the LSN for
> + @return the starting LSN or CDIO_INVALID_LSN on error.
> + */
> + lsn_t cdio_get_track_pregap_lsn(const CdIo_t *p_cdio, track_t i_track);
>
> /*!
> Return the starting MSF (minutes/secs/frames) for track number
> diff -upr -x version.h -x 'libiso9660*pc' -x 'libcdio*pc'
> libcdio-0.80-clean/lib/driver/cdio_private.h
> libcdio-0.80/lib/driver/cdio_private.h
> --- libcdio-0.80-clean/lib/driver/cdio_private.h 2006-01-23
> 15:48:11.000000000 -0500
> +++ libcdio-0.80/lib/driver/cdio_private.h 2008-03-09 21:23:04.000000000
> -0400
> @@ -260,6 +260,13 @@ extern "C" {
> CDIO_INVALID_LBA is returned on error.
> */
> lba_t (*get_track_lba) ( void *p_env, track_t i_track );
> +
> + /*!
> + Return the starting LBA for the pregap for track number
> + i_track in p_env. Tracks numbers start at 1.
> + CDIO_INVALID_LBA is returned on error.
> + */
> + lba_t (*get_track_pregap_lba) ( const void *p_env, track_t i_track );
>
> /*!
> Get format of track.
> diff -upr -x version.h -x 'libiso9660*pc' -x 'libcdio*pc'
> libcdio-0.80-clean/lib/driver/image/bincue.c
> libcdio-0.80/lib/driver/image/bincue.c
> --- libcdio-0.80-clean/lib/driver/image/bincue.c 2006-02-13
> 06:00:53.000000000 -0500
> +++ libcdio-0.80/lib/driver/image/bincue.c 2008-03-10 17:28:43.000000000
> -0400
> @@ -632,7 +632,7 @@ parse_cuefile (_img_private_t *cd, const
> goto err_exit;
> }
> if (cd) {
> - cd->tocent[i].pregap = lba;
> + cd->tocent[i].silence = lba;
> }
> } else {
> goto format_error;
> @@ -672,7 +672,14 @@ parse_cuefile (_img_private_t *cd, const
> track_info_t *this_track=
> &(cd->tocent[cd->gen.i_tracks - cd->gen.i_first_track]);
>
> - if (start_index != 0) {
> + switch (start_index) {
> +
> + case 0:
> + lba += CDIO_PREGAP_SECTORS;
> + this_track->pregap = lba;
> + break;
> +
> + case 1:
> if (!b_first_index_for_track) {
> lba += CDIO_PREGAP_SECTORS;
> cdio_lba_to_msf(lba, &(this_track->start_msf));
> @@ -709,6 +716,10 @@ parse_cuefile (_img_private_t *cd, const
> }
> }
> this_track->num_indices++;
> + break;
> +
> + default:
> + break;
> }
> }
> #endif
> @@ -1163,6 +1174,7 @@ cdio_open_cue (const char *psz_cue_name)
> _funcs.get_track_lba = _get_lba_track_bincue;
> _funcs.get_track_msf = _get_track_msf_image;
> _funcs.get_track_preemphasis = get_track_preemphasis_image,
> + _funcs.get_track_pregap_lba = get_track_pregap_lba_image;
> _funcs.lseek = _lseek_bincue;
> _funcs.read = _read_bincue;
> _funcs.read_audio_sectors = _read_audio_sectors_bincue;
> diff -upr -x version.h -x 'libiso9660*pc' -x 'libcdio*pc'
> libcdio-0.80-clean/lib/driver/image/cdrdao.c
> libcdio-0.80/lib/driver/image/cdrdao.c
> --- libcdio-0.80-clean/lib/driver/image/cdrdao.c 2007-03-05
> 06:49:24.000000000 -0500
> +++ libcdio-0.80/lib/driver/image/cdrdao.c 2008-03-10 14:50:45.000000000
> -0400
> @@ -843,7 +843,7 @@ parse_tocfile (_img_private_t *cd, const
> if (0 <= i) {
> if (NULL != (psz_field = strtok (NULL, " \t\n\r"))) {
> if (NULL != cd)
> - cd->tocent[i].pregap = cdio_mmssff_to_lba (psz_field);
> + cd->tocent[i].silence = cdio_mmssff_to_lba (psz_field);
> } else {
> goto format_error;
> }
> diff -upr -x version.h -x 'libiso9660*pc' -x 'libcdio*pc'
> libcdio-0.80-clean/lib/driver/image/nrg.c libcdio-0.80/lib/driver/image/nrg.c
> --- libcdio-0.80-clean/lib/driver/image/nrg.c 2008-03-03
> 07:04:34.000000000 -0500
> +++ libcdio-0.80/lib/driver/image/nrg.c 2008-03-10 15:23:42.000000000
> -0400
> @@ -371,24 +371,24 @@ parse_nrg (_img_private_t *p_env, const
> case DAOX_ID: /* "DAOX" */
> case DAOI_ID: /* "DAOI" */
> {
> + _daox_t *_xentries = NULL;
> + _daoi_t *_ientries = NULL;
> + _dao_common_t *_dao_common = (void *) chunk->data;
> + int disc_mode = _dao_common->unknown[1];
> track_format_t track_format;
> - int disc_mode;
> + int i;
>
> /* We include an extra 0 byte so these can be used as C strings.*/
> - p_env->psz_mcn = calloc(1, CDIO_MCN_SIZE+1);
> + p_env->psz_mcn = calloc(1, CDIO_MCN_SIZE+1);
> + memcpy(p_env->psz_mcn, &(_dao_common->psz_mcn), CDIO_MCN_SIZE);
> + p_env->psz_mcn[CDIO_MCN_SIZE] = '\0';
>
> if (DAOX_ID == opcode) {
> - _daox_array_t *_entries = (void *) chunk->data;
> - disc_mode = _entries->_unknown[1];
> - p_env->dtyp = _entries->_unknown[19];
> - memcpy(p_env->psz_mcn, &(_entries->psz_mcn), CDIO_MCN_SIZE);
> - p_env->psz_mcn[CDIO_MCN_SIZE] = '\0';
> + _xentries = (void *) chunk->data;
> + p_env->dtyp = _xentries->track_info[0].common.unknown[0];
> } else {
> - _daoi_array_t *_entries = (void *) chunk->data;
> - disc_mode = _entries->_unknown[1];
> - p_env->dtyp = _entries->_unknown[19];
> - memcpy(p_env->psz_mcn, &(_entries->psz_mcn), CDIO_MCN_SIZE);
> - p_env->psz_mcn[CDIO_MCN_SIZE] = '\0';
> + _ientries = (void *) chunk->data;
> + p_env->dtyp = _ientries->track_info[0].common.unknown[0];
> }
>
> p_env->is_dao = true;
> @@ -430,7 +430,6 @@ parse_nrg (_img_private_t *p_env, const
> track_format = TRACK_FORMAT_AUDIO;
> }
> if (0 == disc_mode) {
> - int i;
> for (i=0; i<p_env->gen.i_tracks; i++) {
> cdtext_init (&(p_env->gen.cdtext_track[i]));
> p_env->tocent[i].track_format= track_format;
> @@ -446,7 +445,6 @@ parse_nrg (_img_private_t *p_env, const
> }
> }
> } else if (2 == disc_mode) {
> - int i;
> for (i=0; i<p_env->gen.i_tracks; i++) {
> cdtext_init (&(p_env->gen.cdtext_track[i]));
> p_env->tocent[i].track_green = true;
> @@ -473,6 +471,18 @@ parse_nrg (_img_private_t *p_env, const
> "Don't know if mode 1, mode 2 or mixed: %x\n",
> disc_mode);
> }
> +
> + for (i=0; i<p_env->gen.i_tracks; i++) {
> + if (!p_env->tocent[i].datasize) {
> + continue;
> + }
> + if (DAOX_ID == opcode) {
> + p_env->tocent[i].pregap =
> (uint64_from_be(_xentries->track_info[i].index0)) /
> (p_env->tocent[i].datasize);
> + } else {
> + p_env->tocent[i].pregap =
> (uint32_from_be(_ientries->track_info[i].index0)) /
> (p_env->tocent[i].datasize);
> + }
> + }
> +
> break;
> }
> case NERO_ID:
> @@ -1243,13 +1253,14 @@ cdio_open_nrg (const char *psz_source)
> _funcs.get_media_changed = get_media_changed_image;
> _funcs.get_mcn = _get_mcn_image;
> _funcs.get_num_tracks = _get_num_tracks_image;
> - _funcs.get_track_channels = get_track_channels_generic,
> - _funcs.get_track_copy_permit = get_track_copy_permit_image,
> + _funcs.get_track_channels = get_track_channels_generic;
> + _funcs.get_track_copy_permit = get_track_copy_permit_image;
> _funcs.get_track_format = get_track_format_nrg;
> _funcs.get_track_green = _get_track_green_nrg;
> _funcs.get_track_lba = NULL; /* Will use generic routine via msf
> */
> _funcs.get_track_msf = _get_track_msf_image;
> - _funcs.get_track_preemphasis = get_track_preemphasis_generic,
> + _funcs.get_track_preemphasis = get_track_preemphasis_generic;
> + _funcs.get_track_pregap_lba = get_track_pregap_lba_image;
> _funcs.lseek = _lseek_nrg;
> _funcs.read = _read_nrg;
> _funcs.read_audio_sectors = _read_audio_sectors_nrg;
> diff -upr -x version.h -x 'libiso9660*pc' -x 'libcdio*pc'
> libcdio-0.80-clean/lib/driver/image/nrg.h libcdio-0.80/lib/driver/image/nrg.h
> --- libcdio-0.80-clean/lib/driver/image/nrg.h 2006-01-14
> 04:45:44.000000000 -0500
> +++ libcdio-0.80/lib/driver/image/nrg.h 2008-03-08 23:16:21.000000000
> -0500
> @@ -71,19 +71,48 @@ typedef struct {
> uint32_t lsn GNUC_PACKED;
> } _cuex_array_t;
>
> +/* New DAO[XI] Information from
> http://en.wikipedia.org/wiki/NRG_(file_format)
> +*/
> +
> typedef struct {
> - uint32_t _unknown1 GNUC_PACKED;
> - char psz_mcn[CDIO_MCN_SIZE];
> - uint8_t _unknown[64-CDIO_MCN_SIZE-sizeof(uint32_t)];
> + uint8_t zero[10];
> + uint32_t sector_size GNUC_PACKED;
> + uint8_t unknown[4];
> +} _dao_array_common_t;
> +
> +typedef struct {
> + _dao_array_common_t common;
> + uint64_t index0 GNUC_PACKED;
> + uint64_t index1 GNUC_PACKED;
> + uint64_t end_of_track GNUC_PACKED;
> } _daox_array_t;
>
> typedef struct {
> - uint32_t _unknown1 GNUC_PACKED;
> - char psz_mcn[CDIO_MCN_SIZE];
> - uint8_t _unknown[64-CDIO_MCN_SIZE-sizeof(uint32_t)];
> + _dao_array_common_t common;
> + uint32_t index0 GNUC_PACKED;
> + uint32_t index1 GNUC_PACKED;
> + uint32_t end_of_track GNUC_PACKED;
> } _daoi_array_t;
>
> typedef struct {
> + uint32_t chunk_size_le GNUC_PACKED;
> + char psz_mcn[CDIO_MCN_SIZE];
> + uint8_t unknown[3];
> + uint8_t first_track;
> + uint8_t last_track;
> +} _dao_common_t;
> +
> +typedef struct {
> + _dao_common_t common;
> + _daox_array_t track_info[EMPTY_ARRAY_SIZE];
> +} _daox_t;
> +
> +typedef struct {
> + _dao_common_t common;
> + _daoi_array_t track_info[EMPTY_ARRAY_SIZE];
> +} _daoi_t;
> +
> +typedef struct {
> uint32_t id GNUC_PACKED;
> uint32_t len GNUC_PACKED;
> char data[EMPTY_ARRAY_SIZE];
> diff -upr -x version.h -x 'libiso9660*pc' -x 'libcdio*pc'
> libcdio-0.80-clean/lib/driver/image.h libcdio-0.80/lib/driver/image.h
> --- libcdio-0.80-clean/lib/driver/image.h 2005-02-16 23:57:21.000000000
> -0500
> +++ libcdio-0.80/lib/driver/image.h 2008-03-10 14:50:01.000000000 -0400
> @@ -48,7 +48,8 @@ typedef struct {
> msf_t start_msf;
> lba_t start_lba;
> int start_index;
> - lba_t pregap; /**< pre-gap with zero audio data */
> + lba_t pregap; /**< pre-gap */
> + lba_t silence; /**< pre-gap with zero audio data */
> int sec_count; /**< Number of sectors in this track. Does
> not
> include pregap */
> int num_indices;
> diff -upr -x version.h -x 'libiso9660*pc' -x 'libcdio*pc'
> libcdio-0.80-clean/lib/driver/image_common.c
> libcdio-0.80/lib/driver/image_common.c
> --- libcdio-0.80-clean/lib/driver/image_common.c 2005-02-17
> 02:03:37.000000000 -0500
> +++ libcdio-0.80/lib/driver/image_common.c 2008-03-10 15:34:08.000000000
> -0400
> @@ -246,6 +246,30 @@ get_track_preemphasis_image(const void *
> & PRE_EMPHASIS ) ? CDIO_TRACK_FLAG_TRUE : CDIO_TRACK_FLAG_FALSE;
> }
>
> +/*! Return the starting LBA for the pregap for track number i_track.
> + Track numbers start at 1.
> + CDIO_INVALID_LBA is returned on error.
> +*/
> +lba_t
> +get_track_pregap_lba_image(const void *p_user_data, track_t i_track)
> +{
> + const _img_private_t *p_env = p_user_data;
> + lba_t pregap, start_lba;
> +
> + pregap = p_env->tocent[i_track-p_env->gen.i_first_track].pregap;
> + start_lba = p_env->tocent[i_track-p_env->gen.i_first_track].start_lba;
> +
> + /* avoid initializing pregap to CDIO_INVALID_LBA by letting calloc
> + do the work. also, nero files have the pregap set equal
> + to the start of the track when there is no pregap
> + */
> + if (!pregap || pregap == start_lba) {
> + pregap = CDIO_INVALID_LBA;
> + }
> +
> + return pregap;
> +}
> +
> /*!
> Read a data sector
>
> diff -upr -x version.h -x 'libiso9660*pc' -x 'libcdio*pc'
> libcdio-0.80-clean/lib/driver/image_common.h
> libcdio-0.80/lib/driver/image_common.h
> --- libcdio-0.80-clean/lib/driver/image_common.h 2005-02-17
> 02:03:37.000000000 -0500
> +++ libcdio-0.80/lib/driver/image_common.h 2008-03-09 21:03:31.000000000
> -0400
> @@ -150,6 +150,13 @@ track_flag_t get_track_copy_permit_image
> */
> track_flag_t get_track_preemphasis_image(const void *p_user_data,
> track_t i_track);
> +
> +/*! Return the starting LBA for the pregap for track number i_track.
> + Track numbers start at 1.
> + CDIO_INVALID_LBA is returned on error.
> +*/
> +lba_t get_track_pregap_lba_image(const void *p_user_data, track_t i_track);
> +
> /*!
> Read a data sector
>
> diff -upr -x version.h -x 'libiso9660*pc' -x 'libcdio*pc'
> libcdio-0.80-clean/lib/driver/libcdio.sym libcdio-0.80/lib/driver/libcdio.sym
> --- libcdio-0.80-clean/lib/driver/libcdio.sym 2007-12-15
> 17:35:53.000000000 -0500
> +++ libcdio-0.80/lib/driver/libcdio.sym 2008-03-10 16:17:27.000000000
> -0400
> @@ -80,6 +80,8 @@ cdio_get_track_format
> cdio_get_track_green
> cdio_get_track_last_lsn
> cdio_get_track_lba
> +cdio_get_track_pregap_lba
> +cdio_get_track_pregap_lsn
> cdio_get_track_lsn
> cdio_get_track_msf
> cdio_get_track_preemphasis
> diff -upr -x version.h -x 'libiso9660*pc' -x 'libcdio*pc'
> libcdio-0.80-clean/lib/driver/track.c libcdio-0.80/lib/driver/track.c
> --- libcdio-0.80-clean/lib/driver/track.c 2005-02-05 23:20:25.000000000
> -0500
> +++ libcdio-0.80/lib/driver/track.c 2008-03-10 16:16:54.000000000 -0400
> @@ -229,7 +229,7 @@ cdio_get_track_lba(const CdIo_t *p_cdio,
> i_track in cdio. Tracks numbers start at 1.
> The "leadout" track is specified either by
> using i_track LEADOUT_TRACK or the total tracks+1.
> - CDIO_INVALID_LBA is returned on error.
> + CDIO_INVALID_LSN is returned on error.
> */
> lsn_t
> cdio_get_track_lsn(const CdIo_t *p_cdio, track_t i_track)
> @@ -247,6 +247,34 @@ cdio_get_track_lsn(const CdIo_t *p_cdio,
> }
>
> /*!
> + Return the starting LBA for the pregap for track number
> + i_track in cdio. Track numbers start at 1.
> + CDIO_INVALID_LBA is returned on error.
> +*/
> +lba_t
> +cdio_get_track_pregap_lba(const CdIo_t *p_cdio, track_t i_track)
> +{
> + if (p_cdio == NULL) return CDIO_INVALID_LBA;
> +
> + if (p_cdio->op.get_track_pregap_lba) {
> + return p_cdio->op.get_track_pregap_lba (p_cdio->env, i_track);
> + } else {
> + return CDIO_INVALID_LBA;
> + }
> +}
> +
> +/*!
> + Return the starting LSN for the pregap for track number
> + i_track in cdio. Track numbers start at 1.
> + CDIO_INVALID_LSN is returned on error.
> +*/
> +lsn_t
> +cdio_get_track_pregap_lsn(const CdIo_t *p_cdio, track_t i_track)
> +{
> + return cdio_lba_to_lsn(cdio_get_track_pregap_lba(p_cdio, i_track));
> +}
> +
> +/*!
> Return the ending LSN for track number
> i_track in cdio. CDIO_INVALID_LSN is returned on error.
> */
- [Libcdio-devel] libcdio freeze for 0.80 next weekend, R. Bernstein, 2008/03/08
- [Libcdio-devel] here's a patch that changes nothing..., Robert William Fuller, 2008/03/08
- [Libcdio-devel] Re: here's a patch that changes nothing..., Robert William Fuller, 2008/03/08
- [Libcdio-devel] Re: here's a patch that changes nothing..., Robert William Fuller, 2008/03/08
- [Libcdio-devel] Re: here's a patch that changes nothing..., Robert William Fuller, 2008/03/08
- [Libcdio-devel] Re: here's a patch that changes nothing..., Robert William Fuller, 2008/03/08
- [Libcdio-devel] Re: here's a patch that changes nothing..., Robert William Fuller, 2008/03/08
- [Libcdio-devel] Re: here's a patch that changes nothing..., Robert William Fuller, 2008/03/08
- [Libcdio-devel] Re: here's a patch that changes nothing..., R. Bernstein, 2008/03/09
Re: [Libcdio-devel] libcdio freeze for 0.80 next weekend, Robert William Fuller, 2008/03/10
- Re: [Libcdio-devel] libcdio freeze for 0.80 next weekend,
R. Bernstein <=
[Libcdio-devel] proposed pre-gap patch, Robert William Fuller, 2008/03/13
[Libcdio-devel] unexpected offset in bincue.c for audio sectors, Robert William Fuller, 2008/03/17