From e9a544fa98f5e99b3ada9d473530056d8640dbd9 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 1 Mar 2016 21:22:42 +0100 Subject: configure.ac: prepare for 0.19.14 --- NEWS | 2 ++ configure.ac | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 96d8ec8bd..a2a9d2523 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,5 @@ +ver 0.19.14 (not yet released) + ver 0.19.13 (2016/02/23) * tags - aiff, riff: fix ID3 chunk padding diff --git a/configure.ac b/configure.ac index 39f2aaf0c..66621e6a0 100644 --- a/configure.ac +++ b/configure.ac @@ -1,10 +1,10 @@ AC_PREREQ(2.60) -AC_INIT(mpd, 0.19.13, musicpd-dev-team@lists.sourceforge.net) +AC_INIT(mpd, 0.19.14, musicpd-dev-team@lists.sourceforge.net) VERSION_MAJOR=0 VERSION_MINOR=19 -VERSION_REVISION=13 +VERSION_REVISION=14 VERSION_EXTRA=0 AC_CONFIG_SRCDIR([src/Main.cxx]) -- cgit v1.2.3 From d2dd6f7c709f70f325cad34a4bf283d80e832939 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 25 Aug 2015 12:46:12 +0200 Subject: thread/Posix{Mutex,Cond}: use "constexpr" only with glibc Apparently all other C libraries are not compatible with "constexpr". Those which are not will get a performance penalty, but at least they work at all. --- NEWS | 1 + src/thread/PosixCond.hxx | 14 +++++++------- src/thread/PosixMutex.hxx | 14 +++++++------- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/NEWS b/NEWS index a2a9d2523..cf3e788fc 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,5 @@ ver 0.19.14 (not yet released) +* fix build failures on non-glibc builds due to constexpr Mutex ver 0.19.13 (2016/02/23) * tags diff --git a/src/thread/PosixCond.hxx b/src/thread/PosixCond.hxx index b3fe204e1..1d4a78985 100644 --- a/src/thread/PosixCond.hxx +++ b/src/thread/PosixCond.hxx @@ -41,9 +41,13 @@ class PosixCond { pthread_cond_t cond; public: -#if defined(__NetBSD__) || defined(__BIONIC__) - /* NetBSD's PTHREAD_COND_INITIALIZER is not compatible with - "constexpr" */ +#ifdef __GLIBC__ + /* optimized constexpr constructor for pthread implementations + that support it */ + constexpr PosixCond():cond(PTHREAD_COND_INITIALIZER) {} +#else + /* slow fallback for pthread implementations that are not + compatible with "constexpr" */ PosixCond() { pthread_cond_init(&cond, nullptr); } @@ -51,10 +55,6 @@ public: ~PosixCond() { pthread_cond_destroy(&cond); } -#else - /* optimized constexpr constructor for sane POSIX - implementations */ - constexpr PosixCond():cond(PTHREAD_COND_INITIALIZER) {} #endif PosixCond(const PosixCond &other) = delete; diff --git a/src/thread/PosixMutex.hxx b/src/thread/PosixMutex.hxx index 5805158d5..6afe850f0 100644 --- a/src/thread/PosixMutex.hxx +++ b/src/thread/PosixMutex.hxx @@ -41,9 +41,13 @@ class PosixMutex { pthread_mutex_t mutex; public: -#if defined(__NetBSD__) || defined(__BIONIC__) - /* NetBSD's PTHREAD_MUTEX_INITIALIZER is not compatible with - "constexpr" */ +#ifdef __GLIBC__ + /* optimized constexpr constructor for pthread implementations + that support it */ + constexpr PosixMutex():mutex(PTHREAD_MUTEX_INITIALIZER) {} +#else + /* slow fallback for pthread implementations that are not + compatible with "constexpr" */ PosixMutex() { pthread_mutex_init(&mutex, nullptr); } @@ -51,10 +55,6 @@ public: ~PosixMutex() { pthread_mutex_destroy(&mutex); } -#else - /* optimized constexpr constructor for sane POSIX - implementations */ - constexpr PosixMutex():mutex(PTHREAD_MUTEX_INITIALIZER) {} #endif PosixMutex(const PosixMutex &other) = delete; -- cgit v1.2.3 From bbda335e021c3a4d6ccfce7916041d4fb089af5c Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 6 Mar 2016 23:20:26 +0100 Subject: mixer/pulse: fix integer division rounding --- src/mixer/plugins/PulseMixerPlugin.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mixer/plugins/PulseMixerPlugin.cxx b/src/mixer/plugins/PulseMixerPlugin.cxx index c5f20723b..e0f6407b0 100644 --- a/src/mixer/plugins/PulseMixerPlugin.cxx +++ b/src/mixer/plugins/PulseMixerPlugin.cxx @@ -218,7 +218,7 @@ PulseMixer::SetVolume(unsigned new_volume, Error &error) struct pa_cvolume cvolume; pa_cvolume_set(&cvolume, volume.channels, - (pa_volume_t)new_volume * PA_VOLUME_NORM / 100 + 0.5); + (new_volume * PA_VOLUME_NORM + 50) / 100); bool success = pulse_output_set_volume(output, &cvolume, error); if (success) volume = cvolume; -- cgit v1.2.3 From 976fdd76c1c2213ed89e69d3b1ef8378f1cacbf5 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 6 Mar 2016 23:26:48 +0100 Subject: decoder/opus: limit tag size to 64 kB --- NEWS | 2 ++ src/decoder/plugins/OpusReader.hxx | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index cf3e788fc..416af1159 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,6 @@ ver 0.19.14 (not yet released) +* decoder + - opus: limit tag size to 64 kB * fix build failures on non-glibc builds due to constexpr Mutex ver 0.19.13 (2016/02/23) diff --git a/src/decoder/plugins/OpusReader.hxx b/src/decoder/plugins/OpusReader.hxx index c5b8e9107..219f3f42a 100644 --- a/src/decoder/plugins/OpusReader.hxx +++ b/src/decoder/plugins/OpusReader.hxx @@ -85,7 +85,7 @@ public: char *ReadString() { uint32_t length; - if (!ReadWord(length)) + if (!ReadWord(length) || length >= 65536) return nullptr; const char *src = (const char *)Read(length); -- cgit v1.2.3 From b24cbc68ba10c643cae1bf45e405a3d90f802cf4 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 6 Mar 2016 23:28:29 +0100 Subject: decoder/dsdiff: fix off-by-one buffer overflow --- NEWS | 1 + src/decoder/plugins/DsdiffDecoderPlugin.cxx | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 416af1159..1d6762a54 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,6 @@ ver 0.19.14 (not yet released) * decoder + - dsdiff: fix off-by-one buffer overflow - opus: limit tag size to 64 kB * fix build failures on non-glibc builds due to constexpr Mutex diff --git a/src/decoder/plugins/DsdiffDecoderPlugin.cxx b/src/decoder/plugins/DsdiffDecoderPlugin.cxx index b6c79e11e..99530975d 100644 --- a/src/decoder/plugins/DsdiffDecoderPlugin.cxx +++ b/src/decoder/plugins/DsdiffDecoderPlugin.cxx @@ -205,7 +205,7 @@ dsdiff_handle_native_tag(InputStream &is, if (length == 0 || length > 60) return; - char string[length]; + char string[length + 1]; char *label; label = string; -- cgit v1.2.3 From 1532ffe2159ec10d2ff4a77d6177a077b7e31c2a Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 6 Mar 2016 23:41:08 +0100 Subject: protocol/ArgParser: fix range check The old check unsigned(value) > std::numeric_limits::max() .. cannot ever fail. --- src/protocol/ArgParser.cxx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/protocol/ArgParser.cxx b/src/protocol/ArgParser.cxx index e373827b4..bdc1b95d1 100644 --- a/src/protocol/ArgParser.cxx +++ b/src/protocol/ArgParser.cxx @@ -92,7 +92,7 @@ check_range(Client &client, unsigned *value_r1, unsigned *value_r2, return false; } - if (unsigned(value) > std::numeric_limits::max()) { + if (value > std::numeric_limits::max()) { command_error(client, ACK_ERROR_ARG, "Number too large: %s", s); return false; @@ -117,7 +117,7 @@ check_range(Client &client, unsigned *value_r1, unsigned *value_r2, return false; } - if (unsigned(value) > std::numeric_limits::max()) { + if (value > std::numeric_limits::max()) { command_error(client, ACK_ERROR_ARG, "Number too large: %s", s); return false; -- cgit v1.2.3 From 13f9f0315ff1406f559cea48f2dac7080fedfba2 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 6 Mar 2016 23:53:41 +0100 Subject: util/HugeAllocator: fix division by zero due to inverted check There were two ways this could fail: 1. division by zero when sysconf(_SC_PAGESIZE)==0 2. mmap() failure because the size parameter is not aligned to page size Neither ever happened: sysconf() never fails, and the only caller passes a size that is already aligned. Phew. --- src/util/HugeAllocator.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/HugeAllocator.cxx b/src/util/HugeAllocator.cxx index 1da049f2f..f26ffc814 100644 --- a/src/util/HugeAllocator.cxx +++ b/src/util/HugeAllocator.cxx @@ -46,7 +46,7 @@ static size_t AlignToPageSize(size_t size) { static const long page_size = sysconf(_SC_PAGESIZE); - if (page_size > 0) + if (page_size == 0) return size; size_t ps(page_size); -- cgit v1.2.3 From e44c0254f7ba3d52fea70e9f003a646a831ab573 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 7 Mar 2016 13:15:07 +0100 Subject: archive/iso9660: make variables more local --- src/archive/plugins/Iso9660ArchivePlugin.cxx | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/archive/plugins/Iso9660ArchivePlugin.cxx b/src/archive/plugins/Iso9660ArchivePlugin.cxx index ba415d3c5..c149d0d7e 100644 --- a/src/archive/plugins/Iso9660ArchivePlugin.cxx +++ b/src/archive/plugins/Iso9660ArchivePlugin.cxx @@ -86,18 +86,17 @@ static constexpr Domain iso9660_domain("iso9660"); inline void Iso9660ArchiveFile::Visit(const char *psz_path, ArchiveVisitor &visitor) { - CdioList_t *entlist; - CdioListNode_t *entnode; - iso9660_stat_t *statbuf; char pathname[4096]; - entlist = iso9660_ifs_readdir (iso, psz_path); + auto *entlist = iso9660_ifs_readdir (iso, psz_path); if (!entlist) { return; } /* Iterate over the list of nodes that iso9660_ifs_readdir gives */ + CdioListNode_t *entnode; _CDIO_LIST_FOREACH (entnode, entlist) { - statbuf = (iso9660_stat_t *) _cdio_list_node_data (entnode); + auto *statbuf = (iso9660_stat_t *) + _cdio_list_node_data(entnode); strcpy(pathname, psz_path); strcat(pathname, statbuf->filename); -- cgit v1.2.3 From 065a9ed10f7a8600e867879b3e9c683df283c7b0 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 7 Mar 2016 13:57:07 +0100 Subject: archive/iso9660: add local variable "filename" --- src/archive/plugins/Iso9660ArchivePlugin.cxx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/archive/plugins/Iso9660ArchivePlugin.cxx b/src/archive/plugins/Iso9660ArchivePlugin.cxx index c149d0d7e..6c71f9ba0 100644 --- a/src/archive/plugins/Iso9660ArchivePlugin.cxx +++ b/src/archive/plugins/Iso9660ArchivePlugin.cxx @@ -97,12 +97,13 @@ Iso9660ArchiveFile::Visit(const char *psz_path, ArchiveVisitor &visitor) _CDIO_LIST_FOREACH (entnode, entlist) { auto *statbuf = (iso9660_stat_t *) _cdio_list_node_data(entnode); + const char *filename = statbuf->filename; strcpy(pathname, psz_path); - strcat(pathname, statbuf->filename); + strcat(pathname, filename); if (iso9660_stat_s::_STAT_DIR == statbuf->type ) { - if (strcmp(statbuf->filename, ".") && strcmp(statbuf->filename, "..")) { + if (strcmp(filename, ".") && strcmp(filename, "..")) { strcat(pathname, "/"); Visit(pathname, visitor); } -- cgit v1.2.3 From c46fc4531b10d6b53a646897e76559a2e3817ec7 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 7 Mar 2016 14:01:40 +0100 Subject: archive/iso9660: move the "." and ".." checks up --- src/archive/plugins/Iso9660ArchivePlugin.cxx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/archive/plugins/Iso9660ArchivePlugin.cxx b/src/archive/plugins/Iso9660ArchivePlugin.cxx index 6c71f9ba0..f56c608b1 100644 --- a/src/archive/plugins/Iso9660ArchivePlugin.cxx +++ b/src/archive/plugins/Iso9660ArchivePlugin.cxx @@ -98,15 +98,15 @@ Iso9660ArchiveFile::Visit(const char *psz_path, ArchiveVisitor &visitor) auto *statbuf = (iso9660_stat_t *) _cdio_list_node_data(entnode); const char *filename = statbuf->filename; + if (strcmp(filename, ".") == 0 || strcmp(filename, "..") == 0) + continue; strcpy(pathname, psz_path); strcat(pathname, filename); if (iso9660_stat_s::_STAT_DIR == statbuf->type ) { - if (strcmp(filename, ".") && strcmp(filename, "..")) { - strcat(pathname, "/"); - Visit(pathname, visitor); - } + strcat(pathname, "/"); + Visit(pathname, visitor); } else { //remove leading / visitor.VisitArchiveEntry(pathname + 1); -- cgit v1.2.3 From de61c3b9626d24685e58f880667001129e5b8921 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 7 Mar 2016 14:00:13 +0100 Subject: archive/iso9660: use a single path buffer for Visit() Avoid wasting 4 kB stack per directory level. --- src/archive/plugins/Iso9660ArchivePlugin.cxx | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/archive/plugins/Iso9660ArchivePlugin.cxx b/src/archive/plugins/Iso9660ArchivePlugin.cxx index f56c608b1..24609f1ed 100644 --- a/src/archive/plugins/Iso9660ArchivePlugin.cxx +++ b/src/archive/plugins/Iso9660ArchivePlugin.cxx @@ -66,7 +66,8 @@ public: return iso9660_iso_seek_read(iso, ptr, start, i_size); } - void Visit(const char *path, ArchiveVisitor &visitor); + void Visit(char *path, size_t length, + ArchiveVisitor &visitor); virtual void Close() override { Unref(); @@ -84,11 +85,10 @@ static constexpr Domain iso9660_domain("iso9660"); /* archive open && listing routine */ inline void -Iso9660ArchiveFile::Visit(const char *psz_path, ArchiveVisitor &visitor) +Iso9660ArchiveFile::Visit(char *path, size_t length, + ArchiveVisitor &visitor) { - char pathname[4096]; - - auto *entlist = iso9660_ifs_readdir (iso, psz_path); + auto *entlist = iso9660_ifs_readdir(iso, path); if (!entlist) { return; } @@ -101,15 +101,16 @@ Iso9660ArchiveFile::Visit(const char *psz_path, ArchiveVisitor &visitor) if (strcmp(filename, ".") == 0 || strcmp(filename, "..") == 0) continue; - strcpy(pathname, psz_path); - strcat(pathname, filename); + size_t filename_length = strlen(filename); + memcpy(path + length, filename, filename_length + 1); + size_t new_length = length + filename_length; if (iso9660_stat_s::_STAT_DIR == statbuf->type ) { - strcat(pathname, "/"); - Visit(pathname, visitor); + memcpy(path + new_length, "/", 2); + Visit(path, new_length + 1, visitor); } else { //remove leading / - visitor.VisitArchiveEntry(pathname + 1); + visitor.VisitArchiveEntry(path + 1); } } _cdio_list_free (entlist, true); @@ -133,7 +134,8 @@ iso9660_archive_open(Path pathname, Error &error) void Iso9660ArchiveFile::Visit(ArchiveVisitor &visitor) { - Visit("/", visitor); + char path[4096] = "/"; + Visit(path, 1, visitor); } /* single archive handling */ -- cgit v1.2.3 From e140a28073f75375a33b28bcbcd0ecc669c5a518 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 7 Mar 2016 14:18:39 +0100 Subject: archive/iso9660: check path buffer bounds --- NEWS | 2 ++ src/archive/plugins/Iso9660ArchivePlugin.cxx | 15 +++++++++++---- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/NEWS b/NEWS index 1d6762a54..62ea0c08e 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,8 @@ ver 0.19.14 (not yet released) * decoder - dsdiff: fix off-by-one buffer overflow - opus: limit tag size to 64 kB +* archive + - iso9660: fix buffer overflow * fix build failures on non-glibc builds due to constexpr Mutex ver 0.19.13 (2016/02/23) diff --git a/src/archive/plugins/Iso9660ArchivePlugin.cxx b/src/archive/plugins/Iso9660ArchivePlugin.cxx index 24609f1ed..6b5f47e86 100644 --- a/src/archive/plugins/Iso9660ArchivePlugin.cxx +++ b/src/archive/plugins/Iso9660ArchivePlugin.cxx @@ -66,7 +66,10 @@ public: return iso9660_iso_seek_read(iso, ptr, start, i_size); } - void Visit(char *path, size_t length, + /** + * @param capacity the path buffer size + */ + void Visit(char *path, size_t length, size_t capacity, ArchiveVisitor &visitor); virtual void Close() override { @@ -85,7 +88,7 @@ static constexpr Domain iso9660_domain("iso9660"); /* archive open && listing routine */ inline void -Iso9660ArchiveFile::Visit(char *path, size_t length, +Iso9660ArchiveFile::Visit(char *path, size_t length, size_t capacity, ArchiveVisitor &visitor) { auto *entlist = iso9660_ifs_readdir(iso, path); @@ -102,12 +105,16 @@ Iso9660ArchiveFile::Visit(char *path, size_t length, continue; size_t filename_length = strlen(filename); + if (length + filename_length + 1 >= capacity) + /* file name is too long */ + continue; + memcpy(path + length, filename, filename_length + 1); size_t new_length = length + filename_length; if (iso9660_stat_s::_STAT_DIR == statbuf->type ) { memcpy(path + new_length, "/", 2); - Visit(path, new_length + 1, visitor); + Visit(path, new_length + 1, capacity, visitor); } else { //remove leading / visitor.VisitArchiveEntry(path + 1); @@ -135,7 +142,7 @@ void Iso9660ArchiveFile::Visit(ArchiveVisitor &visitor) { char path[4096] = "/"; - Visit(path, 1, visitor); + Visit(path, 1, sizeof(path), visitor); } /* single archive handling */ -- cgit v1.2.3