From 17dd21ac7fc0667121f72e6ddc0a2b90d33b28ad Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@musicpd.org>
Date: Mon, 7 Sep 2020 13:12:08 +0200
Subject: [PATCH] archive/iso9660: fix unaligned reads

Oh the horror!  This plugin cannot possibly ever have worked.  It was
broken from the start, when it was added in commit 37796699cf7 nearly
twelve (!) years ago.

The plugin would always read at sector boundaries, so it could only
ever work at multiples of 2 kB.
---
 NEWS                                         |   1 +
 src/archive/plugins/Iso9660ArchivePlugin.cxx | 109 +++++++++++++++----
 2 files changed, 88 insertions(+), 22 deletions(-)

diff --git a/NEWS b/NEWS
index bbfbb271f..b80e5b19c 100644
--- a/NEWS
+++ b/NEWS
@@ -5,6 +5,7 @@ ver 0.21.26 (not yet released)
 * archive
   - bzip2: fix crash on corrupt bzip2 file
   - bzip2: flush output at end of input file
+  - iso9660: fix unaligned reads
   - zzip: fix crash on corrupt ZIP file
 * decoder
   - sndfile: fix lost samples at end of file
diff --git a/src/archive/plugins/Iso9660ArchivePlugin.cxx b/src/archive/plugins/Iso9660ArchivePlugin.cxx
index 6314fb025..f81f6fd98 100644
--- a/src/archive/plugins/Iso9660ArchivePlugin.cxx
+++ b/src/archive/plugins/Iso9660ArchivePlugin.cxx
@@ -29,9 +29,12 @@
 #include "fs/Path.hxx"
 #include "util/RuntimeError.hxx"
 #include "util/StringCompare.hxx"
+#include "util/WritableBuffer.hxx"
 
 #include <cdio/iso9660.h>
 
+#include <array>
+
 #include <stdlib.h>
 #include <string.h>
 
@@ -144,6 +147,47 @@ class Iso9660InputStream final : public InputStream {
 
 	const lsn_t lsn;
 
+	/**
+	 * libiso9660 can only read whole sectors at a time, and this
+	 * buffer is used to store one whole sector and allow Read()
+	 * to handle partial sector reads.
+	 */
+	class BlockBuffer {
+		size_t position = 0, fill = 0;
+
+		std::array<uint8_t, ISO_BLOCKSIZE> data;
+
+	public:
+		ConstBuffer<uint8_t> Read() const noexcept {
+			assert(fill <= data.size());
+			assert(position <= fill);
+
+			return {&data[position], &data[fill]};
+		}
+
+		void Consume(size_t nbytes) noexcept {
+			assert(nbytes <= Read().size);
+
+			position += nbytes;
+		}
+
+		WritableBuffer<uint8_t> Write() noexcept {
+			assert(Read().empty());
+
+			return {data.data(), data.size()};
+		}
+
+		void Append(size_t nbytes) noexcept {
+			assert(Read().empty());
+			assert(nbytes <= data.size());
+
+			fill = nbytes;
+			position = 0;
+		}
+	};
+
+	BlockBuffer buffer;
+
 public:
 	Iso9660InputStream(const std::shared_ptr<Iso9660> &_iso,
 			   const char *_uri,
@@ -182,35 +226,56 @@ Iso9660ArchiveFile::OpenStream(const char *pathname,
 size_t
 Iso9660InputStream::Read(void *ptr, size_t read_size)
 {
-	const ScopeUnlock unlock(mutex);
-
-	int readed = 0;
-	int no_blocks, cur_block;
-	size_t left_bytes = size - offset;
-
-	if (left_bytes < read_size) {
-		no_blocks = CEILING(left_bytes, ISO_BLOCKSIZE);
-	} else {
-		no_blocks = read_size / ISO_BLOCKSIZE;
-	}
-
-	if (no_blocks == 0)
+	const offset_type remaining = size - offset;
+	if (remaining == 0)
 		return 0;
 
-	cur_block = offset / ISO_BLOCKSIZE;
+	if (offset_type(read_size) > remaining)
+		read_size = remaining;
 
-	readed = iso->SeekRead(ptr, lsn + cur_block, no_blocks);
+	auto r = buffer.Read();
 
-	if (readed != no_blocks * ISO_BLOCKSIZE)
-		throw FormatRuntimeError("error reading ISO file at lsn %lu",
-					 (unsigned long)cur_block);
+	if (r.empty()) {
+		/* the buffer is empty - read more data from the ISO file */
 
-	if (left_bytes < read_size) {
-		readed = left_bytes;
+		assert(offset % ISO_BLOCKSIZE == 0);
+
+		const ScopeUnlock unlock(mutex);
+
+		const lsn_t read_lsn = lsn + offset / ISO_BLOCKSIZE;
+
+		if (read_size >= ISO_BLOCKSIZE) {
+			/* big read - read right into the caller's buffer */
+
+			auto nbytes = iso->SeekRead(ptr, read_lsn,
+						    read_size / ISO_BLOCKSIZE);
+			if (nbytes <= 0)
+				throw std::runtime_error("Failed to read ISO9660 file");
+
+			offset += nbytes;
+			return nbytes;
+		}
+
+		/* fill the buffer */
+
+		auto w = buffer.Write();
+		auto nbytes = iso->SeekRead(w.data, read_lsn,
+					    w.size / ISO_BLOCKSIZE);
+		if (nbytes <= 0)
+			throw std::runtime_error("Failed to read ISO9660 file");
+
+		buffer.Append(nbytes);
+
+		r = buffer.Read();
 	}
 
-	offset += readed;
-	return readed;
+	assert(!r.empty());
+
+	size_t nbytes = std::min(read_size, r.size);
+	memcpy(ptr, r.data, nbytes);
+	buffer.Consume(nbytes);
+	offset += nbytes;
+	return nbytes;
 }
 
 bool