From 8adcfdbf41d475b589ff21fc0d77cc164aae0bca Mon Sep 17 00:00:00 2001 From: arvidn Date: Fri, 13 Oct 2017 02:34:24 +0300 Subject: [PATCH] make disk_buffer_holder know the size of the bufer it holds, to fix buffer overrun in chained_buffer fix --- include/libtorrent/chained_buffer.hpp | 8 ++++---- include/libtorrent/disk_buffer_holder.hpp | 15 ++++++++++----- src/block_cache.cpp | 8 ++++---- src/disk_buffer_holder.cpp | 19 ++++++++++++------- src/disk_io_thread.cpp | 6 +++--- test/test_block_cache.cpp | 6 +++--- 6 files changed, 36 insertions(+), 26 deletions(-) diff --git a/include/libtorrent/chained_buffer.hpp b/include/libtorrent/chained_buffer.hpp index c201c8849..128cf5a89 100644 --- a/include/libtorrent/chained_buffer.hpp +++ b/include/libtorrent/chained_buffer.hpp @@ -112,10 +112,10 @@ namespace libtorrent { #if TORRENT_CPP98_DEQUE move_construct_holder_fun move_holder; #endif - aux::aligned_storage<24>::type holder; - char* buf; // the first byte of the buffer - int size; // the total size of the buffer - int used_size; // this is the number of bytes to send/receive + aux::aligned_storage<32>::type holder; + char* buf = nullptr; // the first byte of the buffer + int size = 0; // the total size of the buffer + int used_size = 0; // this is the number of bytes to send/receive }; public: diff --git a/include/libtorrent/disk_buffer_holder.hpp b/include/libtorrent/disk_buffer_holder.hpp index b50de93e1..6d2b879bb 100644 --- a/include/libtorrent/disk_buffer_holder.hpp +++ b/include/libtorrent/disk_buffer_holder.hpp @@ -63,7 +63,8 @@ namespace libtorrent { struct TORRENT_EXTRA_EXPORT disk_buffer_holder { // internal - disk_buffer_holder(buffer_allocator_interface& alloc, char* buf) noexcept; + disk_buffer_holder(buffer_allocator_interface& alloc + , char* buf, std::size_t sz) noexcept; disk_buffer_holder& operator=(disk_buffer_holder&&) noexcept; disk_buffer_holder(disk_buffer_holder&&) noexcept; @@ -75,7 +76,9 @@ namespace libtorrent { // using a disk buffer pool directly (there's only one // disk_buffer_pool per session) disk_buffer_holder(buffer_allocator_interface& alloc - , aux::block_cache_reference const& ref, char* buf) noexcept; + , aux::block_cache_reference const& ref + , char* buf + , std::size_t sz) noexcept; // frees any unreleased disk buffer held by this object ~disk_buffer_holder(); @@ -92,14 +95,15 @@ namespace libtorrent { // set the holder object to hold the specified buffer // (or nullptr by default). If it's already holding a // disk buffer, it will first be freed. - void reset(char* buf = nullptr); - void reset(aux::block_cache_reference const& ref, char* buf); + void reset(char* buf = nullptr, std::size_t sz = 0); + void reset(aux::block_cache_reference const& ref, char* buf, std::size_t sz); // swap pointers of two disk buffer holders. void swap(disk_buffer_holder& h) noexcept { TORRENT_ASSERT(h.m_allocator == m_allocator); std::swap(h.m_buf, m_buf); + std::swap(h.m_size, m_size); std::swap(h.m_ref, m_ref); } @@ -110,12 +114,13 @@ namespace libtorrent { // buffer explicit operator bool() const noexcept { return m_buf != nullptr; } - std::size_t size() const { return 0x4000; } + std::size_t size() const { return m_size; } private: buffer_allocator_interface* m_allocator; char* m_buf; + std::size_t m_size; aux::block_cache_reference m_ref; }; diff --git a/src/block_cache.cpp b/src/block_cache.cpp index 00a3b216b..42c1b180e 100644 --- a/src/block_cache.cpp +++ b/src/block_cache.cpp @@ -1670,10 +1670,11 @@ int block_cache::copy_from_piece(cached_piece_entry* const pe // make sure it didn't wrap TORRENT_PIECE_ASSERT(pe->refcount > 0, pe); int const blocks_per_piece = (j->storage->files().piece_length() + block_size() - 1) / block_size(); + TORRENT_ASSERT(block_offset < 0x4000); j->argument = disk_buffer_holder(allocator , aux::block_cache_reference{ j->storage->storage_index() , static_cast(pe->piece) * blocks_per_piece + start_block} - , bl.buf + (j->d.io.offset & (block_size() - 1))); + , bl.buf + block_offset, static_cast(0x4000 - block_offset)); j->storage->inc_refcount(); ++m_send_buffer_blocks; @@ -1690,14 +1691,13 @@ int block_cache::copy_from_piece(cached_piece_entry* const pe } j->argument = disk_buffer_holder(allocator - , allocate_buffer("send buffer")); + , allocate_buffer("send buffer"), 0x4000); if (!boost::get(j->argument)) return -2; while (size > 0) { TORRENT_PIECE_ASSERT(pe->blocks[block].buf, pe); - int to_copy = (std::min)(block_size() - - block_offset, size); + int to_copy = std::min(block_size() - block_offset, size); std::memcpy(boost::get(j->argument).get() + buffer_offset , pe->blocks[block].buf + block_offset diff --git a/src/disk_buffer_holder.cpp b/src/disk_buffer_holder.cpp index e4291ca3f..74bfbbbd4 100644 --- a/src/disk_buffer_holder.cpp +++ b/src/disk_buffer_holder.cpp @@ -36,8 +36,9 @@ POSSIBILITY OF SUCH DAMAGE. namespace libtorrent { - disk_buffer_holder::disk_buffer_holder(buffer_allocator_interface& alloc, char* buf) noexcept - : m_allocator(&alloc), m_buf(buf), m_ref() + disk_buffer_holder::disk_buffer_holder(buffer_allocator_interface& alloc + , char* buf, std::size_t sz) noexcept + : m_allocator(&alloc), m_buf(buf), m_size(sz), m_ref() {} disk_buffer_holder& disk_buffer_holder::operator=(disk_buffer_holder&& h) noexcept @@ -48,7 +49,7 @@ namespace libtorrent { } disk_buffer_holder::disk_buffer_holder(disk_buffer_holder&& h) noexcept - : m_allocator(h.m_allocator), m_buf(h.m_buf), m_ref(h.m_ref) + : m_allocator(h.m_allocator), m_buf(h.m_buf), m_size(h.m_size), m_ref(h.m_ref) { // we own this buffer now h.m_buf = nullptr; @@ -56,23 +57,26 @@ namespace libtorrent { } disk_buffer_holder::disk_buffer_holder(buffer_allocator_interface& alloc - , aux::block_cache_reference const& ref, char* buf) noexcept - : m_allocator(&alloc), m_buf(buf), m_ref(ref) + , aux::block_cache_reference const& ref, char* buf + , std::size_t sz) noexcept + : m_allocator(&alloc), m_buf(buf), m_size(sz), m_ref(ref) {} - void disk_buffer_holder::reset(aux::block_cache_reference const& ref, char* buf) + void disk_buffer_holder::reset(aux::block_cache_reference const& ref, char* buf, std::size_t const sz) { if (m_ref.cookie != aux::block_cache_reference::none) m_allocator->reclaim_blocks(m_ref); else if (m_buf) m_allocator->free_disk_buffer(m_buf); m_buf = buf; + m_size = sz; m_ref = ref; } - void disk_buffer_holder::reset(char* const buf) + void disk_buffer_holder::reset(char* const buf, std::size_t const sz) { if (m_ref.cookie != aux::block_cache_reference::none) m_allocator->reclaim_blocks(m_ref); else if (m_buf) m_allocator->free_disk_buffer(m_buf); m_buf = buf; + m_size = sz; m_ref = aux::block_cache_reference(); } @@ -81,6 +85,7 @@ namespace libtorrent { TORRENT_ASSERT(m_ref.cookie == aux::block_cache_reference::none); char* ret = m_buf; m_buf = nullptr; + m_size = 0; m_ref = aux::block_cache_reference(); return ret; } diff --git a/src/disk_io_thread.cpp b/src/disk_io_thread.cpp index 849bebee2..c807f5377 100644 --- a/src/disk_io_thread.cpp +++ b/src/disk_io_thread.cpp @@ -1231,7 +1231,7 @@ constexpr disk_job_flags_t disk_interface::cache_hit; status_t disk_io_thread::do_uncached_read(disk_io_job* j) { - j->argument = disk_buffer_holder(*this, m_disk_cache.allocate_buffer("send buffer")); + j->argument = disk_buffer_holder(*this, m_disk_cache.allocate_buffer("send buffer"), 0x4000); auto& buffer = boost::get(j->argument); if (buffer.get() == nullptr) { @@ -1583,7 +1583,7 @@ constexpr disk_job_flags_t disk_interface::cache_hit; j->piece = r.piece; j->d.io.offset = r.start; j->d.io.buffer_size = std::uint16_t(r.length); - j->argument = disk_buffer_holder(*this, nullptr); + j->argument = disk_buffer_holder(*this, nullptr, 0); j->flags = flags; j->callback = std::move(handler); @@ -1688,7 +1688,7 @@ constexpr disk_job_flags_t disk_interface::cache_hit; TORRENT_ASSERT(r.length <= 16 * 1024); bool exceeded = false; - disk_buffer_holder buffer(*this, m_disk_cache.allocate_buffer(exceeded, o, "receive buffer")); + disk_buffer_holder buffer(*this, m_disk_cache.allocate_buffer(exceeded, o, "receive buffer"), 0x4000); if (!buffer) aux::throw_ex(); std::memcpy(buffer.get(), buf, aux::numeric_cast(r.length)); diff --git a/test/test_block_cache.cpp b/test/test_block_cache.cpp index f312e6f31..5e0cd59c6 100644 --- a/test/test_block_cache.cpp +++ b/test/test_block_cache.cpp @@ -140,7 +140,7 @@ static void nop() {} wj.d.io.offset = (b) * 0x4000; \ wj.d.io.buffer_size = 0x4000; \ wj.piece = piece_index_t(p); \ - wj.argument = disk_buffer_holder(alloc, bc.allocate_buffer("write-test")); \ + wj.argument = disk_buffer_holder(alloc, bc.allocate_buffer("write-test"), 0x4000); \ pe = bc.add_dirty_block(&wj) #define READ_BLOCK(p, b, r) \ @@ -149,7 +149,7 @@ static void nop() {} rj.d.io.buffer_size = 0x4000; \ rj.piece = piece_index_t(p); \ rj.storage = pm; \ - rj.argument = disk_buffer_holder(alloc, nullptr); \ + rj.argument = disk_buffer_holder(alloc, nullptr, 0); \ ret = bc.try_read(&rj, alloc) #define FLUSH(flushing) \ @@ -444,7 +444,7 @@ void test_unaligned_read() rj.d.io.buffer_size = 0x4000; rj.piece = piece_index_t(0); rj.storage = pm; - rj.argument = disk_buffer_holder(alloc, nullptr); + rj.argument = disk_buffer_holder(alloc, nullptr, 0); ret = bc.try_read(&rj, alloc); // unaligned reads copies the data into a new buffer