release v0.19.14

When a reference counter is at its limit, don't allocate a new
TagPoolSlot - that would result in many TagPoolSlot instances with
ref==1.  This in turn would make the linked list very very large,
which means quadratic runtime for many operations.

Makefile.am

@@ -1219,6 +1219,7 @@ liboutput_plugins_a_SOURCES = \
 
 MIXER_LIBS = \
 	libmixer_plugins.a \
+	$(ALSA_LIBS) \
 	$(PULSE_LIBS)
 
 MIXER_API_SRC = \

NEWS

@@ -1,3 +1,25 @@
+ver 0.19.14 (2016/03/18)
+* decoder
+  - dsdiff: fix off-by-one buffer overflow
+  - opus: limit tag size to 64 kB
+* archive
+  - iso9660: fix buffer overflow
+* fix quadratic runtime bug in the tag pool
+* fix build failures on non-glibc builds due to constexpr Mutex
+
+ver 0.19.13 (2016/02/23)
+* tags
+  - aiff, riff: fix ID3 chunk padding
+* decoder
+  - ffmpeg: support the TAK codec
+* fix disappearing duration of remote songs during playback
+* initialize supplementary groups with glibc 2.19+
+
+ver 0.19.12 (2015/12/15)
+* fix assertion failure on malformed UTF-8 tag
+* fix build failure on non-Linux systems
+* fix LimitRTTIME in systemd unit file
+
 ver 0.19.11 (2015/10/27)
 * tags
   - ape: fix buffer overflow

configure.ac

@@ -1,10 +1,10 @@
 AC_PREREQ(2.60)
 
-AC_INIT(mpd, 0.19.11, 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=11
+VERSION_REVISION=14
 VERSION_EXTRA=0
 
 AC_CONFIG_SRCDIR([src/Main.cxx])
@@ -206,6 +206,7 @@ if test x$host_is_linux = xyes; then
 fi
 
 AC_CHECK_FUNCS(getpwnam_r getpwuid_r)
+AC_CHECK_FUNCS(initgroups)
 AC_CHECK_FUNCS(strndup)
 
 if test x$host_is_linux = xyes; then

doc/user.xml

@@ -2647,7 +2647,8 @@ buffer_size: 16384</programlisting>
           / <ulink
           url="http://icecast.org/"><application>IceCast</application></ulink>.
           HTTP streaming clients like
-          <application>mplayer</application> can connect to it.
+          <application>mplayer</application>, <application>VLC</application>,
+          and <application>mpv</application> can connect to it.
         </para>
 
         <para>

src/DetachedSong.hxx

@@ -188,6 +188,14 @@ public:
 		tag = std::move(other.tag);
 	}
 
+	/**
+	 * Similar to the MoveTagFrom(), but move only the #TagItem
+	 * array.
+	 */
+	void MoveTagItemsFrom(DetachedSong &&other) {
+		tag.MoveItemsFrom(std::move(other.tag));
+	}
+
 	time_t GetLastModified() const {
 		return mtime;
 	}

src/Main.cxx

@@ -630,7 +630,7 @@ static int mpd_main_after_fork(struct options options)
 					 config_get_unsigned(CONF_AUTO_UPDATE_DEPTH,
 							     INT_MAX));
 #else
-		FormatWarning(main_domain,
+		FormatWarning(config_domain,
 			      "inotify: auto_update was disabled. enable during compilation phase");
 #endif
 	}

src/archive/plugins/Iso9660ArchivePlugin.cxx

@@ -66,7 +66,11 @@ public:
 		return iso9660_iso_seek_read(iso, ptr, start, i_size);
 	}
 
-	void Visit(const char *path, ArchiveVisitor &visitor);
+	/**
+	 * @param capacity the path buffer size
+	 */
+	void Visit(char *path, size_t length, size_t capacity,
+		   ArchiveVisitor &visitor);
 
 	virtual void Close() override {
 		Unref();
@@ -84,32 +88,36 @@ 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, size_t capacity,
+			  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, 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);
+		const char *filename = statbuf->filename;
+		if (strcmp(filename, ".") == 0 || strcmp(filename, "..") == 0)
+			continue;
 
-		strcpy(pathname, psz_path);
-		strcat(pathname, statbuf->filename);
+		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 ) {
-			if (strcmp(statbuf->filename, ".") && strcmp(statbuf->filename, "..")) {
-				strcat(pathname, "/");
-				Visit(pathname, visitor);
-			}
+			memcpy(path + new_length, "/", 2);
+			Visit(path, new_length + 1, capacity, visitor);
 		} else {
 			//remove leading /
-			visitor.VisitArchiveEntry(pathname + 1);
+			visitor.VisitArchiveEntry(path + 1);
 		}
 	}
 	_cdio_list_free (entlist, true);
@@ -133,7 +141,8 @@ iso9660_archive_open(Path pathname, Error &error)
 void
 Iso9660ArchiveFile::Visit(ArchiveVisitor &visitor)
 {
-	Visit("/", visitor);
+	char path[4096] = "/";
+	Visit(path, 1, sizeof(path), visitor);
 }
 
 /* single archive handling */

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;
 

src/decoder/plugins/FfmpegDecoderPlugin.cxx

@@ -738,7 +738,7 @@ static const char *const ffmpeg_suffixes[] = {
 	"mve", "mvi", "mxf", "nc", "nsv", "nut", "nuv", "oga", "ogm", "ogv",
 	"ogx", "oma", "ogg", "omg", "opus", "psp", "pva", "qcp", "qt", "r3d", "ra",
 	"ram", "rl2", "rm", "rmvb", "roq", "rpl", "rvc", "shn", "smk", "snd",
-	"sol", "son", "spx", "str", "swf", "tgi", "tgq", "tgv", "thp", "ts",
+	"sol", "son", "spx", "str", "swf", "tak", "tgi", "tgq", "tgv", "thp", "ts",
 	"tsp", "tta", "xa", "xvid", "uv", "uv2", "vb", "vid", "vob", "voc",
 	"vp6", "vmd", "wav", "webm", "wma", "wmv", "wsaud", "wsvga", "wv",
 	"wve",

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);

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;

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<unsigned>::max()) {
+	if (value > std::numeric_limits<int>::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<unsigned>::max()) {
+		if (value > std::numeric_limits<int>::max()) {
 			command_error(client, ACK_ERROR_ARG,
 				      "Number too large: %s", s);
 			return false;

src/queue/Playlist.cxx

@@ -37,7 +37,7 @@ playlist::TagModified(DetachedSong &&song)
 
 	DetachedSong &current_song = queue.GetOrder(current);
 	if (song.IsSame(current_song))
-		current_song.MoveTagFrom(std::move(song));
+		current_song.MoveTagItemsFrom(std::move(song));
 
 	queue.ModifyAtOrder(current);
 	queue.IncrementVersion();

src/tag/Aiff.cxx

@@ -84,14 +84,14 @@ aiff_seek_id3(FILE *file)
 			   underflow when casting to off_t */
 			return 0;
 
-		if (size % 2 != 0)
-			/* pad byte */
-			++size;
-
 		if (memcmp(chunk.id, "ID3 ", 4) == 0)
 			/* found it! */
 			return size;
 
+		if (size % 2 != 0)
+			/* pad byte */
+			++size;
+
 		if (fseek(file, size, SEEK_CUR) != 0)
 			return 0;
 	}

src/tag/Riff.cxx

@@ -82,15 +82,15 @@ riff_seek_id3(FILE *file)
 			   underflow when casting to off_t */
 			return 0;
 
-		if (size % 2 != 0)
-			/* pad byte */
-			++size;
-
 		if (memcmp(chunk.id, "id3 ", 4) == 0 ||
 		    memcmp(chunk.id, "ID3 ", 4) == 0)
 			/* found it! */
 			return size;
 
+		if (size % 2 != 0)
+			/* pad byte */
+			++size;
+
 		if (fseek(file, size, SEEK_CUR) != 0)
 			return 0;
 	}

src/tag/Tag.hxx

@@ -80,9 +80,17 @@ struct Tag {
 	Tag &operator=(Tag &&other) {
 		duration = other.duration;
 		has_playlist = other.has_playlist;
+		MoveItemsFrom(std::move(other));
+		return *this;
+	}
+
+	/**
+	 * Similar to the move operator, but move only the #TagItem
+	 * array.
+	 */
+	void MoveItemsFrom(Tag &&other) {
 		std::swap(items, other.items);
 		std::swap(num_items, other.num_items);
-		return *this;
 	}
 
 	/**

src/tag/TagPool.cxx

@@ -23,19 +23,23 @@
 #include "util/Cast.hxx"
 #include "util/VarSize.hxx"
 
+#include <limits>
+
 #include <assert.h>
 #include <string.h>
 #include <stdlib.h>
 
 Mutex tag_pool_lock;
 
-static constexpr size_t NUM_SLOTS = 4096;
+static constexpr size_t NUM_SLOTS = 4093;
 
 struct TagPoolSlot {
 	TagPoolSlot *next;
 	unsigned char ref;
 	TagItem item;
 
+	static constexpr unsigned MAX_REF = std::numeric_limits<decltype(ref)>::max();
+
 	TagPoolSlot(TagPoolSlot *_next, TagType type,
 		    const char *value, size_t length)
 		:next(_next), ref(1) {
@@ -116,7 +120,7 @@ tag_pool_get_item(TagType type, const char *value, size_t length)
 		if (slot->item.type == type &&
 		    length == strlen(slot->item.value) &&
 		    memcmp(value, slot->item.value, length) == 0 &&
-		    slot->ref < 0xff) {
+		    slot->ref < TagPoolSlot::MAX_REF) {
 			assert(slot->ref > 0);
 			++slot->ref;
 			return &slot->item;
@@ -135,19 +139,15 @@ tag_pool_dup_item(TagItem *item)
 
 	assert(slot->ref > 0);
 
-	if (slot->ref < 0xff) {
+	if (slot->ref < TagPoolSlot::MAX_REF) {
 		++slot->ref;
 		return item;
 	} else {
-		/* the reference counter overflows above 0xff;
-		   duplicate the item, and start with 1 */
+		/* the reference counter overflows above MAX_REF;
+		   obtain a reference to a different TagPoolSlot which
+		   isn't yet "full" */
 		size_t length = strlen(item->value);
-		auto slot_p = tag_value_slot_p(item->type,
-					       item->value, length);
-		slot = TagPoolSlot::Create(*slot_p, item->type,
-					   item->value, strlen(item->value));
-		*slot_p = slot;
-		return &slot->item;
+		return tag_pool_get_item(item->type, item->value, length);
 	}
 }
 

src/tag/TagString.cxx

@@ -40,9 +40,9 @@ FindInvalidUTF8(const char *p, const char *const end)
 		/* now call the other SequenceLengthUTF8() overload
 		   which also validates the continuations */
 		const size_t t = SequenceLengthUTF8(p);
-		assert(s == t);
 		if (t == 0)
 			return p;
+		assert(s == t);
 
 		p += s;
 	}

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;

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;

src/unix/Daemon.cxx

@@ -110,7 +110,7 @@ daemonize_set_user(void)
 				       (int)user_gid);
 	}
 
-#ifdef _BSD_SOURCE
+#ifdef HAVE_INITGROUPS
 	/* init supplementary groups
 	 * (must be done before we change our uid)
 	 */

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);

systemd/mpd.service.in

@@ -7,7 +7,7 @@ ExecStart=@prefix@/bin/mpd --no-daemon
 
 # allow MPD to use real-time priority 50
 LimitRTPRIO=50
-LimitRTTIME=-1
+LimitRTTIME=infinity
 
 [Install]
 WantedBy=multi-user.target