From 54889c72e3469027a852d9e8ff029d659e612094 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 23 Jul 2009 12:01:03 +0200 Subject: pcm_convert: use GError for error handling Don't abort the whole MPD process when the conversion fails. This has been a denial-of-service attack vector for years. --- src/decoder_api.c | 18 +++--- src/filter/convert_filter_plugin.c | 14 +---- src/pcm_channels.c | 20 +------ src/pcm_convert.c | 109 ++++++++++++++++++++++++++----------- src/pcm_convert.h | 13 ++++- src/pcm_format.c | 5 -- src/pcm_resample.c | 18 ++++-- src/pcm_resample.h | 16 +++--- src/pcm_resample_internal.h | 8 +-- src/pcm_resample_libsamplerate.c | 71 ++++++++++++++++-------- test/run_output.c | 5 +- 11 files changed, 181 insertions(+), 116 deletions(-) diff --git a/src/decoder_api.c b/src/decoder_api.c index 408e8005e..7f66c881e 100644 --- a/src/decoder_api.c +++ b/src/decoder_api.c @@ -225,6 +225,7 @@ decoder_data(struct decoder *decoder, struct replay_gain_info *replay_gain_info) { const char *data = _data; + GError *error = NULL; assert(dc.state == DECODE_STATE_DECODE); assert(dc.pipe != NULL); @@ -259,14 +260,15 @@ decoder_data(struct decoder *decoder, if (!audio_format_equals(&dc.in_audio_format, &dc.out_audio_format)) { data = pcm_convert(&decoder->conv_state, &dc.in_audio_format, data, length, - &dc.out_audio_format, &length); - - /* under certain circumstances, pcm_convert() may - return an empty buffer - this condition should be - investigated further, but for now, do this check as - a workaround: */ - if (data == NULL) - return DECODE_COMMAND_NONE; + &dc.out_audio_format, &length, + &error); + if (data == NULL) { + /* the PCM conversion has failed - stop + playback, since we have no better way to + bail out */ + g_warning("%s", error->message); + return DECODE_COMMAND_STOP; + } } while (length > 0) { diff --git a/src/filter/convert_filter_plugin.c b/src/filter/convert_filter_plugin.c index b7f16de4f..d197dbdb9 100644 --- a/src/filter/convert_filter_plugin.c +++ b/src/filter/convert_filter_plugin.c @@ -53,12 +53,6 @@ struct convert_filter { struct pcm_convert_state state; }; -static inline GQuark -convert_quark(void) -{ - return g_quark_from_static_string("pcm_convert"); -} - static struct filter * convert_filter_init(G_GNUC_UNUSED const struct config_param *param, G_GNUC_UNUSED GError **error_r) @@ -119,12 +113,10 @@ convert_filter_filter(struct filter *_filter, const void *src, size_t src_size, dest = pcm_convert(&filter->state, &filter->in_audio_format, src, src_size, - &filter->out_audio_format, dest_size_r); - if (dest == NULL) { - g_set_error(error_r, convert_quark(), 0, - "pcm_convert() has failed"); + &filter->out_audio_format, dest_size_r, + error_r); + if (dest == NULL) return NULL; - } return dest; } diff --git a/src/pcm_channels.c b/src/pcm_channels.c index 4f9343912..38445f958 100644 --- a/src/pcm_channels.c +++ b/src/pcm_channels.c @@ -20,13 +20,8 @@ #include "pcm_channels.h" #include "pcm_buffer.h" -#include - #include -#undef G_LOG_DOMAIN -#define G_LOG_DOMAIN "pcm" - static void pcm_convert_channels_16_1_to_2(int16_t *dest, const int16_t *src, unsigned num_frames) @@ -92,11 +87,8 @@ pcm_convert_channels_16(struct pcm_buffer *buffer, else if (dest_channels == 2) pcm_convert_channels_16_n_to_2(dest, src_channels, src, num_frames); - else { - g_warning("conversion %u->%u channels is not supported", - src_channels, dest_channels); + else return NULL; - } return dest; } @@ -166,11 +158,8 @@ pcm_convert_channels_24(struct pcm_buffer *buffer, else if (dest_channels == 2) pcm_convert_channels_24_n_to_2(dest, src_channels, src, num_frames); - else { - g_warning("conversion %u->%u channels is not supported", - src_channels, dest_channels); + else return NULL; - } return dest; } @@ -235,11 +224,8 @@ pcm_convert_channels_32(struct pcm_buffer *buffer, else if (dest_channels == 2) pcm_convert_channels_32_n_to_2(dest, src_channels, src, num_frames); - else { - g_warning("conversion %u->%u channels is not supported", - src_channels, dest_channels); + else return NULL; - } return dest; } diff --git a/src/pcm_convert.c b/src/pcm_convert.c index 8a35d807e..4879dc0ab 100644 --- a/src/pcm_convert.c +++ b/src/pcm_convert.c @@ -56,8 +56,8 @@ static const int16_t * pcm_convert_16(struct pcm_convert_state *state, const struct audio_format *src_format, const void *src_buffer, size_t src_size, - const struct audio_format *dest_format, - size_t *dest_size_r) + const struct audio_format *dest_format, size_t *dest_size_r, + GError **error_r) { const int16_t *buf; size_t len; @@ -67,24 +67,37 @@ pcm_convert_16(struct pcm_convert_state *state, buf = pcm_convert_to_16(&state->format_buffer, &state->dither, src_format->bits, src_buffer, src_size, &len); - if (!buf) - g_error("pcm_convert_to_16() failed"); + if (buf == NULL) { + g_set_error(error_r, pcm_convert_quark(), 0, + "Conversion from %u to 16 bit is not implemented", + src_format->bits); + return NULL; + } if (src_format->channels != dest_format->channels) { buf = pcm_convert_channels_16(&state->channels_buffer, dest_format->channels, src_format->channels, buf, len, &len); - if (!buf) - g_error("pcm_convert_channels_16() failed"); + if (buf == NULL) { + g_set_error(error_r, pcm_convert_quark(), 0, + "Conversion from %u to %u channels " + "is not implemented", + src_format->channels, + dest_format->channels); + return NULL; + } } - if (src_format->sample_rate != dest_format->sample_rate) + if (src_format->sample_rate != dest_format->sample_rate) { buf = pcm_resample_16(&state->resample, dest_format->channels, src_format->sample_rate, buf, len, - dest_format->sample_rate, - &len); + dest_format->sample_rate, &len, + error_r); + if (buf == NULL) + return NULL; + } if (dest_format->reverse_endian) { buf = pcm_byteswap_16(&state->byteswap_buffer, buf, len); @@ -99,8 +112,8 @@ static const int32_t * pcm_convert_24(struct pcm_convert_state *state, const struct audio_format *src_format, const void *src_buffer, size_t src_size, - const struct audio_format *dest_format, - size_t *dest_size_r) + const struct audio_format *dest_format, size_t *dest_size_r, + GError **error_r) { const int32_t *buf; size_t len; @@ -109,24 +122,37 @@ pcm_convert_24(struct pcm_convert_state *state, buf = pcm_convert_to_24(&state->format_buffer, src_format->bits, src_buffer, src_size, &len); - if (!buf) - g_error("pcm_convert_to_24() failed"); + if (buf == NULL) { + g_set_error(error_r, pcm_convert_quark(), 0, + "Conversion from %u to 24 bit is not implemented", + src_format->bits); + return NULL; + } if (src_format->channels != dest_format->channels) { buf = pcm_convert_channels_24(&state->channels_buffer, dest_format->channels, src_format->channels, buf, len, &len); - if (!buf) - g_error("pcm_convert_channels_24() failed"); + if (buf == NULL) { + g_set_error(error_r, pcm_convert_quark(), 0, + "Conversion from %u to %u channels " + "is not implemented", + src_format->channels, + dest_format->channels); + return NULL; + } } - if (src_format->sample_rate != dest_format->sample_rate) + if (src_format->sample_rate != dest_format->sample_rate) { buf = pcm_resample_24(&state->resample, dest_format->channels, src_format->sample_rate, buf, len, - dest_format->sample_rate, - &len); + dest_format->sample_rate, &len, + error_r); + if (buf == NULL) + return NULL; + } if (dest_format->reverse_endian) { buf = pcm_byteswap_32(&state->byteswap_buffer, buf, len); @@ -141,8 +167,8 @@ static const int32_t * pcm_convert_32(struct pcm_convert_state *state, const struct audio_format *src_format, const void *src_buffer, size_t src_size, - const struct audio_format *dest_format, - size_t *dest_size_r) + const struct audio_format *dest_format, size_t *dest_size_r, + GError **error_r) { const int32_t *buf; size_t len; @@ -151,24 +177,37 @@ pcm_convert_32(struct pcm_convert_state *state, buf = pcm_convert_to_32(&state->format_buffer, src_format->bits, src_buffer, src_size, &len); - if (!buf) - g_error("pcm_convert_to_32() failed"); + if (buf == NULL) { + g_set_error(error_r, pcm_convert_quark(), 0, + "Conversion from %u to 24 bit is not implemented", + src_format->bits); + return NULL; + } if (src_format->channels != dest_format->channels) { buf = pcm_convert_channels_32(&state->channels_buffer, dest_format->channels, src_format->channels, buf, len, &len); - if (!buf) - g_error("pcm_convert_channels_32() failed"); + if (buf == NULL) { + g_set_error(error_r, pcm_convert_quark(), 0, + "Conversion from %u to %u channels " + "is not implemented", + src_format->channels, + dest_format->channels); + return NULL; + } } - if (src_format->sample_rate != dest_format->sample_rate) + if (src_format->sample_rate != dest_format->sample_rate) { buf = pcm_resample_32(&state->resample, dest_format->channels, src_format->sample_rate, buf, len, - dest_format->sample_rate, - &len); + dest_format->sample_rate, &len, + error_r); + if (buf == NULL) + return buf; + } if (dest_format->reverse_endian) { buf = pcm_byteswap_32(&state->byteswap_buffer, buf, len); @@ -184,26 +223,32 @@ pcm_convert(struct pcm_convert_state *state, const struct audio_format *src_format, const void *src, size_t src_size, const struct audio_format *dest_format, - size_t *dest_size_r) + size_t *dest_size_r, + GError **error_r) { switch (dest_format->bits) { case 16: return pcm_convert_16(state, src_format, src, src_size, - dest_format, dest_size_r); + dest_format, dest_size_r, + error_r); case 24: return pcm_convert_24(state, src_format, src, src_size, - dest_format, dest_size_r); + dest_format, dest_size_r, + error_r); case 32: return pcm_convert_32(state, src_format, src, src_size, - dest_format, dest_size_r); + dest_format, dest_size_r, + error_r); default: - g_error("cannot convert to %u bit\n", dest_format->bits); + g_set_error(error_r, pcm_convert_quark(), 0, + "PCM conversion to %u bit is not implemented", + dest_format->bits); return NULL; } } diff --git a/src/pcm_convert.h b/src/pcm_convert.h index a048d6598..7ef0782df 100644 --- a/src/pcm_convert.h +++ b/src/pcm_convert.h @@ -46,6 +46,12 @@ struct pcm_convert_state { struct pcm_buffer byteswap_buffer; }; +static inline GQuark +pcm_convert_quark(void) +{ + return g_quark_from_static_string("pcm_convert"); +} + /** * Initializes a pcm_convert_state object. */ @@ -66,13 +72,16 @@ void pcm_convert_deinit(struct pcm_convert_state *state); * @param src_size the size of #src in bytes * @param dest_format the requested destination audio format * @param dest_size_r returns the number of bytes of the destination buffer - * @return the destination buffer + * @param error_r location to store the error occuring, or NULL to + * ignore errors + * @return the destination buffer, or NULL on error */ const void * pcm_convert(struct pcm_convert_state *state, const struct audio_format *src_format, const void *src, size_t src_size, const struct audio_format *dest_format, - size_t *dest_size_r); + size_t *dest_size_r, + GError **error_r); #endif diff --git a/src/pcm_format.c b/src/pcm_format.c index 0e686e17c..64e5167b5 100644 --- a/src/pcm_format.c +++ b/src/pcm_format.c @@ -21,8 +21,6 @@ #include "pcm_dither.h" #include "pcm_buffer.h" -#include - static void pcm_convert_8_to_16(int16_t *out, const int8_t *in, unsigned num_samples) @@ -93,7 +91,6 @@ pcm_convert_to_16(struct pcm_buffer *buffer, struct pcm_dither *dither, return dest; } - g_warning("only 8 or 16 bits are supported for conversion!\n"); return NULL; } @@ -168,7 +165,6 @@ pcm_convert_to_24(struct pcm_buffer *buffer, return dest; } - g_warning("only 8 or 24 bits are supported for conversion!\n"); return NULL; } @@ -243,6 +239,5 @@ pcm_convert_to_32(struct pcm_buffer *buffer, return src; } - g_warning("only 8 or 32 bits are supported for conversion!\n"); return NULL; } diff --git a/src/pcm_resample.c b/src/pcm_resample.c index 1e4a95ceb..f09c65a32 100644 --- a/src/pcm_resample.c +++ b/src/pcm_resample.c @@ -63,14 +63,17 @@ const int16_t * pcm_resample_16(struct pcm_resample_state *state, uint8_t channels, unsigned src_rate, const int16_t *src_buffer, size_t src_size, - unsigned dest_rate, - size_t *dest_size_r) + unsigned dest_rate, size_t *dest_size_r, + GError **error_r) { #ifdef HAVE_LIBSAMPLERATE if (pcm_resample_lsr_enabled()) return pcm_resample_lsr_16(state, channels, src_rate, src_buffer, src_size, - dest_rate, dest_size_r); + dest_rate, dest_size_r, + error_r); +#else + (void)error_r; #endif return pcm_resample_fallback_16(state, channels, @@ -82,14 +85,17 @@ const int32_t * pcm_resample_32(struct pcm_resample_state *state, uint8_t channels, unsigned src_rate, const int32_t *src_buffer, size_t src_size, - unsigned dest_rate, - size_t *dest_size_r) + unsigned dest_rate, size_t *dest_size_r, + GError **error_r) { #ifdef HAVE_LIBSAMPLERATE if (pcm_resample_lsr_enabled()) return pcm_resample_lsr_32(state, channels, src_rate, src_buffer, src_size, - dest_rate, dest_size_r); + dest_rate, dest_size_r, + error_r); +#else + (void)error_r; #endif return pcm_resample_fallback_32(state, channels, diff --git a/src/pcm_resample.h b/src/pcm_resample.h index 44720f7b2..9d03bbfbf 100644 --- a/src/pcm_resample.h +++ b/src/pcm_resample.h @@ -48,7 +48,7 @@ struct pcm_resample_state { uint8_t channels; } prev; - bool error; + int error; #endif struct pcm_buffer buffer; @@ -82,8 +82,8 @@ pcm_resample_16(struct pcm_resample_state *state, uint8_t channels, unsigned src_rate, const int16_t *src_buffer, size_t src_size, - unsigned dest_rate, - size_t *dest_size_r); + unsigned dest_rate, size_t *dest_size_r, + GError **error_r); /** * Resamples 32 bit PCM data. @@ -102,8 +102,8 @@ pcm_resample_32(struct pcm_resample_state *state, uint8_t channels, unsigned src_rate, const int32_t *src_buffer, size_t src_size, - unsigned dest_rate, - size_t *dest_size_r); + unsigned dest_rate, size_t *dest_size_r, + GError **error_r); /** * Resamples 24 bit PCM data. @@ -122,14 +122,14 @@ pcm_resample_24(struct pcm_resample_state *state, uint8_t channels, unsigned src_rate, const int32_t *src_buffer, size_t src_size, - unsigned dest_rate, - size_t *dest_size_r) + unsigned dest_rate, size_t *dest_size_r, + GError **error_r) { /* reuse the 32 bit code - the resampler code doesn't care if the upper 8 bits are actually used */ return pcm_resample_32(state, channels, src_rate, src_buffer, src_size, - dest_rate, dest_size_r); + dest_rate, dest_size_r, error_r); } #endif diff --git a/src/pcm_resample_internal.h b/src/pcm_resample_internal.h index a10ba08cd..74363a590 100644 --- a/src/pcm_resample_internal.h +++ b/src/pcm_resample_internal.h @@ -40,8 +40,8 @@ pcm_resample_lsr_16(struct pcm_resample_state *state, uint8_t channels, unsigned src_rate, const int16_t *src_buffer, size_t src_size, - unsigned dest_rate, - size_t *dest_size_r); + unsigned dest_rate, size_t *dest_size_r, + GError **error_r); const int32_t * pcm_resample_lsr_32(struct pcm_resample_state *state, @@ -49,8 +49,8 @@ pcm_resample_lsr_32(struct pcm_resample_state *state, unsigned src_rate, const int32_t *src_buffer, G_GNUC_UNUSED size_t src_size, - unsigned dest_rate, - size_t *dest_size_r); + unsigned dest_rate, size_t *dest_size_r, + GError **error_r); #endif diff --git a/src/pcm_resample_libsamplerate.c b/src/pcm_resample_libsamplerate.c index 6d019e892..66a1c3193 100644 --- a/src/pcm_resample_libsamplerate.c +++ b/src/pcm_resample_libsamplerate.c @@ -30,6 +30,12 @@ #undef G_LOG_DOMAIN #define G_LOG_DOMAIN "pcm" +static inline GQuark +libsamplerate_quark(void) +{ + return g_quark_from_static_string("libsamplerate"); +} + void pcm_resample_lsr_deinit(struct pcm_resample_state *state) { @@ -77,9 +83,10 @@ out: return convalgo; } -static void +static bool pcm_resample_set(struct pcm_resample_state *state, - uint8_t channels, unsigned src_rate, unsigned dest_rate) + uint8_t channels, unsigned src_rate, unsigned dest_rate, + GError **error_r) { static int convalgo = -1; int error; @@ -92,9 +99,9 @@ pcm_resample_set(struct pcm_resample_state *state, if (channels == state->prev.channels && src_rate == state->prev.src_rate && dest_rate == state->prev.dest_rate) - return; + return true; - state->error = false; + state->error = 0; state->prev.channels = channels; state->prev.src_rate = src_rate; state->prev.dest_rate = dest_rate; @@ -104,16 +111,18 @@ pcm_resample_set(struct pcm_resample_state *state, state->state = src_new(convalgo, channels, &error); if (!state->state) { - g_warning("cannot create new libsamplerate state: %s", - src_strerror(error)); - state->error = true; - return; + g_set_error(error_r, libsamplerate_quark(), state->error, + "libsamplerate initialization has failed: %s", + src_strerror(error)); + return false; } data->src_ratio = (double)dest_rate / (double)src_rate; g_debug("setting samplerate conversion ratio to %.2lf", data->src_ratio); src_set_ratio(state->state, data->src_ratio); + + return true; } const int16_t * @@ -121,9 +130,10 @@ pcm_resample_lsr_16(struct pcm_resample_state *state, uint8_t channels, unsigned src_rate, const int16_t *src_buffer, size_t src_size, - unsigned dest_rate, - size_t *dest_size_r) + unsigned dest_rate, size_t *dest_size_r, + GError **error_r) { + bool success; SRC_DATA *data = &state->data; size_t data_in_size; size_t data_out_size; @@ -132,11 +142,18 @@ pcm_resample_lsr_16(struct pcm_resample_state *state, assert((src_size % (sizeof(*src_buffer) * channels)) == 0); - pcm_resample_set(state, channels, src_rate, dest_rate); + success = pcm_resample_set(state, channels, src_rate, dest_rate, + error_r); + if (!success) + return NULL; /* there was an error previously, and nothing has changed */ - if (state->error) + if (state->error) { + g_set_error(error_r, libsamplerate_quark(), state->error, + "libsamplerate has failed: %s", + src_strerror(state->error)); return NULL; + } data->input_frames = src_size / sizeof(*src_buffer) / channels; data_in_size = data->input_frames * sizeof(float) * channels; @@ -151,9 +168,10 @@ pcm_resample_lsr_16(struct pcm_resample_state *state, error = src_process(state->state, data); if (error) { - g_warning("error processing samples with libsamplerate: %s", - src_strerror(error)); - state->error = true; + g_set_error(error_r, libsamplerate_quark(), error, + "libsamplerate has failed: %s", + src_strerror(error)); + state->error = error; return NULL; } @@ -191,9 +209,10 @@ pcm_resample_lsr_32(struct pcm_resample_state *state, uint8_t channels, unsigned src_rate, const int32_t *src_buffer, size_t src_size, - unsigned dest_rate, - size_t *dest_size_r) + unsigned dest_rate, size_t *dest_size_r, + GError **error_r) { + bool success; SRC_DATA *data = &state->data; size_t data_in_size; size_t data_out_size; @@ -202,11 +221,18 @@ pcm_resample_lsr_32(struct pcm_resample_state *state, assert((src_size % (sizeof(*src_buffer) * channels)) == 0); - pcm_resample_set(state, channels, src_rate, dest_rate); + success = pcm_resample_set(state, channels, src_rate, dest_rate, + error_r); + if (!success) + return NULL; /* there was an error previously, and nothing has changed */ - if (state->error) + if (state->error) { + g_set_error(error_r, libsamplerate_quark(), state->error, + "libsamplerate has failed: %s", + src_strerror(state->error)); return NULL; + } data->input_frames = src_size / sizeof(*src_buffer) / channels; data_in_size = data->input_frames * sizeof(float) * channels; @@ -221,9 +247,10 @@ pcm_resample_lsr_32(struct pcm_resample_state *state, error = src_process(state->state, data); if (error) { - g_warning("error processing samples with libsamplerate: %s", - src_strerror(error)); - state->error = true; + g_set_error(error_r, libsamplerate_quark(), error, + "libsamplerate has failed: %s", + src_strerror(error)); + state->error = error; return NULL; } diff --git a/test/run_output.c b/test/run_output.c index a280f88d4..594c4cd64 100644 --- a/test/run_output.c +++ b/test/run_output.c @@ -44,8 +44,11 @@ pcm_convert(G_GNUC_UNUSED struct pcm_convert_state *state, G_GNUC_UNUSED const struct audio_format *src_format, G_GNUC_UNUSED const void *src, G_GNUC_UNUSED size_t src_size, G_GNUC_UNUSED const struct audio_format *dest_format, - G_GNUC_UNUSED size_t *dest_size_r) + G_GNUC_UNUSED size_t *dest_size_r, + GError **error_r) { + g_set_error(error_r, pcm_convert_quark(), 0, + "Not implemented"); return NULL; } -- cgit v1.2.3