diff --git a/NEWS b/NEWS index 51e520a49..e5fd491d0 100644 --- a/NEWS +++ b/NEWS @@ -27,6 +27,22 @@ ver 0.16 (20??/??/??) * obey $(sysconfdir) for default mpd.conf location +ver 0.15.2 (2009/08/15) +* tags: + - ape: check the tag size (fixes integer underflow) + - ape: added protection against large memory allocations +* decoders: + - mad: skip ID3 frames when libid3tag is disabled + - flac: parse all replaygain tags + - flac: don't allocate cuesheet twice (memleak) +* output: + - shout: fixed stuck pause bug + - shout: minimize the unpause latency +* update: free empty path string (memleak) +* update: free temporary string in container scan (memleak) +* directory: free empty directories after removing them (memleak) + + ver 0.15.1 (2009/07/15) * decoders: - flac: fix assertion failure in tag_free() call diff --git a/configure.ac b/configure.ac index 600b51df0..67b2f2957 100644 --- a/configure.ac +++ b/configure.ac @@ -629,8 +629,7 @@ dnl audio output plugins dnl AC_ARG_ENABLE(alsa, - AS_HELP_STRING([--enable-alsa], - [disable ALSA support]),, + AS_HELP_STRING([--enable-alsa], [enable ALSA support]),, enable_alsa=auto) AC_ARG_ENABLE(ao, diff --git a/src/decoder/_flac_common.c b/src/decoder/_flac_common.c index 7b3453854..09f7269bd 100644 --- a/src/decoder/_flac_common.c +++ b/src/decoder/_flac_common.c @@ -40,53 +40,57 @@ flac_data_init(struct flac_data *data, struct decoder * decoder, data->tag = NULL; } -static bool +static void flac_find_float_comment(const FLAC__StreamMetadata *block, - const char *cmnt, float *fl) + const char *cmnt, float *fl, bool *found_r) { - int offset = - FLAC__metadata_object_vorbiscomment_find_entry_from(block, 0, cmnt); + int offset; + size_t pos; + int len; + unsigned char tmp, *p; - if (offset >= 0) { - size_t pos = strlen(cmnt) + 1; /* 1 is for '=' */ - int len = block->data.vorbis_comment.comments[offset].length - - pos; - if (len > 0) { - unsigned char tmp; - unsigned char *p = &(block->data.vorbis_comment. - comments[offset].entry[pos]); - tmp = p[len]; - p[len] = '\0'; - *fl = (float)atof((char *)p); - p[len] = tmp; + offset = FLAC__metadata_object_vorbiscomment_find_entry_from(block, 0, + cmnt); + if (offset < 0) + return; - return true; - } - } + pos = strlen(cmnt) + 1; /* 1 is for '=' */ + len = block->data.vorbis_comment.comments[offset].length - pos; + if (len <= 0) + return; - return false; + p = &block->data.vorbis_comment.comments[offset].entry[pos]; + tmp = p[len]; + p[len] = '\0'; + *fl = (float)atof((char *)p); + p[len] = tmp; + + *found_r = true; } -/* replaygain stuff by AliasMrJones */ static void flac_parse_replay_gain(const FLAC__StreamMetadata *block, struct flac_data *data) { - bool found; + bool found = false; if (data->replay_gain_info) replay_gain_info_free(data->replay_gain_info); data->replay_gain_info = replay_gain_info_new(); - found = flac_find_float_comment(block, "replaygain_album_gain", - &data->replay_gain_info->tuples[REPLAY_GAIN_ALBUM].gain) || - flac_find_float_comment(block, "replaygain_album_peak", - &data->replay_gain_info->tuples[REPLAY_GAIN_ALBUM].peak) || - flac_find_float_comment(block, "replaygain_track_gain", - &data->replay_gain_info->tuples[REPLAY_GAIN_TRACK].gain) || - flac_find_float_comment(block, "replaygain_track_peak", - &data->replay_gain_info->tuples[REPLAY_GAIN_TRACK].peak); + flac_find_float_comment(block, "replaygain_album_gain", + &data->replay_gain_info->tuples[REPLAY_GAIN_ALBUM].gain, + &found); + flac_find_float_comment(block, "replaygain_album_peak", + &data->replay_gain_info->tuples[REPLAY_GAIN_ALBUM].peak, + &found); + flac_find_float_comment(block, "replaygain_track_gain", + &data->replay_gain_info->tuples[REPLAY_GAIN_TRACK].gain, + &found); + flac_find_float_comment(block, "replaygain_track_peak", + &data->replay_gain_info->tuples[REPLAY_GAIN_TRACK].peak, + &found); if (!found) { replay_gain_info_free(data->replay_gain_info); @@ -106,25 +110,27 @@ flac_comment_value(const FLAC__StreamMetadata_VorbisComment_Entry *entry, size_t char_tnum_length = 0; const char *comment = (const char*)entry->entry; - if (entry->length > name_length && - g_ascii_strncasecmp(comment, name, name_length) == 0) { - if (char_tnum != NULL) { - char_tnum_length = strlen(char_tnum); - if (entry->length > name_length + char_tnum_length + 2 && - comment[name_length] == '[' && - g_ascii_strncasecmp(comment + name_length + 1, - char_tnum, char_tnum_length) == 0 && - comment[name_length + char_tnum_length + 1] == ']') - name_length = name_length + char_tnum_length + 2; - else if (entry->length > name_length + char_tnum_length && - g_ascii_strncasecmp(comment + name_length, - char_tnum, char_tnum_length) == 0) - name_length = name_length + char_tnum_length; - } - if (comment[name_length] == '=') { - *length_r = entry->length - name_length - 1; - return comment + name_length + 1; - } + if (entry->length <= name_length || + g_ascii_strncasecmp(comment, name, name_length) != 0) + return NULL; + + if (char_tnum != NULL) { + char_tnum_length = strlen(char_tnum); + if (entry->length > name_length + char_tnum_length + 2 && + comment[name_length] == '[' && + g_ascii_strncasecmp(comment + name_length + 1, + char_tnum, char_tnum_length) == 0 && + comment[name_length + char_tnum_length + 1] == ']') + name_length = name_length + char_tnum_length + 2; + else if (entry->length > name_length + char_tnum_length && + g_ascii_strncasecmp(comment + name_length, + char_tnum, char_tnum_length) == 0) + name_length = name_length + char_tnum_length; + } + + if (comment[name_length] == '=') { + *length_r = entry->length - name_length - 1; + return comment + name_length + 1; } return NULL; @@ -370,13 +376,15 @@ char* flac_cue_track( const char* pathname, const unsigned int tnum) { - FLAC__StreamMetadata* cs = FLAC__metadata_object_new(FLAC__METADATA_TYPE_CUESHEET); + FLAC__bool success; + FLAC__StreamMetadata* cs; - FLAC__metadata_get_cuesheet(pathname, &cs); - - if (cs == NULL) + success = FLAC__metadata_get_cuesheet(pathname, &cs); + if (!success) return NULL; + assert(cs != NULL); + if (cs->data.cue_sheet.num_tracks <= 1) { FLAC__metadata_object_delete(cs); diff --git a/src/decoder/mad_plugin.c b/src/decoder/mad_plugin.c index 85f4506d2..c5287564f 100644 --- a/src/decoder/mad_plugin.c +++ b/src/decoder/mad_plugin.c @@ -350,11 +350,11 @@ parse_id3_replay_gain_info(struct id3_tag *tag) } #endif -#ifdef HAVE_ID3TAG static void mp3_parse_id3(struct mp3_data *data, size_t tagsize, struct tag **mpd_tag, struct replay_gain_info **replay_gain_info_r) { +#ifdef HAVE_ID3TAG struct id3_tag *id3_tag = NULL; id3_length_t count; id3_byte_t const *id3_data; @@ -418,8 +418,34 @@ static void mp3_parse_id3(struct mp3_data *data, size_t tagsize, id3_tag_delete(id3_tag); g_free(allocated); -} +#else /* !HAVE_ID3TAG */ + (void)mpd_tag; + (void)replay_gain_info_r; + + /* This code is enabled when libid3tag is disabled. Instead + of parsing the ID3 frame, it just skips it. */ + + mad_stream_skip(&data->stream, tagsize); #endif +} + +#ifndef HAVE_ID3TAG +/** + * This function emulates libid3tag when it is disabled. Instead of + * doing a real analyzation of the frame, it just checks whether the + * frame begins with the string "ID3". If so, it returns the full + * length. + */ +static signed long +id3_tag_query(const void *p0, size_t length) +{ + const char *p = p0; + + return length > 3 && memcmp(p, "ID3", 3) == 0 + ? length + : 0; +} +#endif /* !HAVE_ID3TAG */ static enum mp3_action decode_next_frame_header(struct mp3_data *data, G_GNUC_UNUSED struct tag **tag, @@ -433,7 +459,6 @@ decode_next_frame_header(struct mp3_data *data, G_GNUC_UNUSED struct tag **tag, return DECODE_BREAK; } if (mad_header_decode(&data->frame.header, &data->stream)) { -#ifdef HAVE_ID3TAG if ((data->stream).error == MAD_ERROR_LOSTSYNC && (data->stream).this_frame) { signed long tagsize = id3_tag_query((data->stream). @@ -454,7 +479,6 @@ decode_next_frame_header(struct mp3_data *data, G_GNUC_UNUSED struct tag **tag, return DECODE_CONT; } } -#endif if (MAD_RECOVERABLE((data->stream).error)) { return DECODE_SKIP; } else { @@ -493,7 +517,6 @@ decodeNextFrame(struct mp3_data *data) return DECODE_BREAK; } if (mad_frame_decode(&data->frame, &data->stream)) { -#ifdef HAVE_ID3TAG if ((data->stream).error == MAD_ERROR_LOSTSYNC) { signed long tagsize = id3_tag_query((data->stream). this_frame, @@ -506,7 +529,6 @@ decodeNextFrame(struct mp3_data *data) return DECODE_CONT; } } -#endif if (MAD_RECOVERABLE((data->stream).error)) { return DECODE_SKIP; } else { diff --git a/src/directory.c b/src/directory.c index 85c24fd04..ef8c038a3 100644 --- a/src/directory.c +++ b/src/directory.c @@ -73,9 +73,14 @@ directory_prune_empty(struct directory *directory) struct dirvec *dv = &directory->children; for (i = dv->nr; --i >= 0; ) { - directory_prune_empty(dv->base[i]); - if (directory_is_empty(dv->base[i])) - dirvec_delete(dv, dv->base[i]); + struct directory *child = dv->base[i]; + + directory_prune_empty(child); + + if (directory_is_empty(child)) { + dirvec_delete(dv, child); + directory_free(child); + } } if (!dv->nr) dirvec_destroy(dv); diff --git a/src/output/shout_plugin.c b/src/output/shout_plugin.c index 8e091679e..4412d26ff 100644 --- a/src/output/shout_plugin.c +++ b/src/output/shout_plugin.c @@ -448,8 +448,15 @@ my_shout_play(void *data, const void *chunk, size_t size, GError **error) static bool my_shout_pause(void *data) { + struct shout_data *sd = (struct shout_data *)data; static const char silence[1020]; + if (shout_delay(sd->shout_conn) > 500) { + /* cap the latency for unpause */ + g_usleep(500000); + return true; + } + return my_shout_play(data, silence, sizeof(silence), NULL); } diff --git a/src/output_control.c b/src/output_control.c index 70c6d2b1a..ef77bf4fa 100644 --- a/src/output_control.c +++ b/src/output_control.c @@ -77,6 +77,17 @@ audio_output_open(struct audio_output *ao, audio_format_equals(audio_format, &ao->in_audio_format)) { assert(ao->pipe == mp); + if (ao->pause) { + /* unpause with the CANCEL command; this is a + hack, but suits well for forcing the thread + to leave the ao_pause() thread, and we need + to flush the device buffer anyway */ + + /* we're not using audio_output_cancel() here, + because that function is asynchronous */ + ao_command(ao, AO_COMMAND_CANCEL); + } + return true; } diff --git a/src/output_internal.h b/src/output_internal.h index 6ca179287..4eb77cc49 100644 --- a/src/output_internal.h +++ b/src/output_internal.h @@ -86,6 +86,12 @@ struct audio_output { */ bool open; + /** + * Is the device paused? i.e. the output thread is in the + * ao_pause() loop. + */ + bool pause; + /** * If not NULL, the device has failed, and this timer is used * to estimate how long it should stay disabled (unless diff --git a/src/output_thread.c b/src/output_thread.c index c7bd069b1..e1f20e580 100644 --- a/src/output_thread.c +++ b/src/output_thread.c @@ -295,6 +295,7 @@ static void ao_pause(struct audio_output *ao) bool ret; ao_plugin_cancel(ao->plugin, ao->data); + ao->pause = true; ao_command_finished(ao); do { @@ -304,6 +305,8 @@ static void ao_pause(struct audio_output *ao) break; } } while (ao->command == AO_COMMAND_NONE); + + ao->pause = false; } static gpointer audio_output_task(gpointer arg) diff --git a/src/tag_ape.c b/src/tag_ape.c index 4c3f4cf16..e3b848bfc 100644 --- a/src/tag_ape.c +++ b/src/tag_ape.c @@ -22,6 +22,7 @@ #include +#include #include static const char *const ape_tag_names[] = { @@ -95,15 +96,18 @@ tag_ape_load(const char *file) /* find beginning of ape tag */ tagLen = GUINT32_FROM_LE(footer.length); - if (tagLen < sizeof(footer)) + if (tagLen <= sizeof(footer) + 10) + goto fail; + if (tagLen > 1024 * 1024) + /* refuse to load more than one megabyte of tag data */ goto fail; if (fseek(fp, size - tagLen, SEEK_SET)) goto fail; /* read tag into buffer */ tagLen -= sizeof(footer); - if (tagLen <= 0) - goto fail; + assert(tagLen > 10); + buffer = g_malloc(tagLen); if (fread(buffer, 1, tagLen, fp) != tagLen) goto fail; @@ -121,7 +125,7 @@ tag_ape_load(const char *file) /* get the key */ key = p; - while (tagLen - size > 0 && *p != '\0') { + while (tagLen > size && *p != '\0') { p++; tagLen--; } diff --git a/src/update.c b/src/update.c index b857b4d65..6778a5989 100644 --- a/src/update.c +++ b/src/update.c @@ -430,7 +430,7 @@ update_container_file( struct directory* directory, { char* vtrack = NULL; unsigned int tnum = 0; - const char* pathname = map_directory_child_fs(directory, name); + char* pathname = map_directory_child_fs(directory, name); struct directory* contdir = dirvec_find(&directory->children, name); // directory exists already @@ -446,8 +446,10 @@ update_container_file( struct directory* directory, modified = true; } - else + else { + g_free(pathname); return true; + } } contdir = make_subdir(directory, name); @@ -473,6 +475,8 @@ update_container_file( struct directory* directory, g_free(vtrack); } + g_free(pathname); + if (tnum == 1) { delete_directory(contdir); @@ -767,7 +771,6 @@ static void * update_task(void *_path) { if (_path != NULL && !isRootDirectory(_path)) { updatePath((char *)_path); - g_free(_path); } else { struct directory *directory = db_get_root(); struct stat st; @@ -776,6 +779,8 @@ static void * update_task(void *_path) updateDirectory(directory, &st); } + g_free(_path); + if (modified || !db_exists()) db_save();