From 347b7978e44f7f6ac4618b260e367192cd9a7f49 Mon Sep 17 00:00:00 2001 From: Patrick J Cherry Date: Thu, 8 Feb 2018 16:31:28 +0000 Subject: [PATCH 1/3] Discs must be sized in multiples of 512 bytes or odd things happen In #36 some of the odd errors were due to seeks beyond the end of the disc. This was because the disc was "specially crafted" to be 25GB + 1 byte, which doesn't fit into the normal 512 byte sectors expected of a disc. This lead to reads going beyond the end of the disc etc. If a similarly evil disc is used with `losetup`, it just ignores the last bytes of the disc that don't fit into 512 chunks. This is what that patch does, logging an error at the same time. --- src/common/ioutil.c | 9 +++++++++ src/server/serve.c | 9 +++++++++ tests/acceptance/test_serve_mode.rb | 14 ++++++++++++++ 3 files changed, 32 insertions(+) diff --git a/src/common/ioutil.c b/src/common/ioutil.c index 4c1a4bd..1926e86 100644 --- a/src/common/ioutil.c +++ b/src/common/ioutil.c @@ -97,6 +97,15 @@ int open_and_mmap(const char* filename, int* out_fd, uint64_t *out_size, void ** warn("lseek64() failed"); return size; } + + /* If discs are not in multiples of 512, then odd things happen, + * resulting in reads/writes past the ends of files. + */ + if ( size != (size & (~0x1ff))) { + warn("file does not fit into 512-byte sectors; the end of the file will be ignored."); + size = size & (~0x1ff); + } + if (out_size) { *out_size = size; } diff --git a/src/server/serve.c b/src/server/serve.c index d88e9f4..3a86c3a 100644 --- a/src/server/serve.c +++ b/src/server/serve.c @@ -710,6 +710,15 @@ void serve_init_allocation_map(struct server* params) FATAL_IF_NEGATIVE(fd, "Couldn't open %s", params->filename ); size = lseek64( fd, 0, SEEK_END ); + + /* If discs are not in multiples of 512, then odd things happen, + * resulting in reads/writes past the ends of files. + */ + if ( size != (size & ~0x1ff)) { + warn("file does not fit into 512-byte sectors; the end of the file will be ignored."); + size &= ~0x1ff; + } + params->size = size; FATAL_IF_NEGATIVE( size, "Couldn't find size of %s", params->filename ); diff --git a/tests/acceptance/test_serve_mode.rb b/tests/acceptance/test_serve_mode.rb index 4f9a77e..2299584 100644 --- a/tests/acceptance/test_serve_mode.rb +++ b/tests/acceptance/test_serve_mode.rb @@ -195,4 +195,18 @@ class TestServeMode < Test::Unit::TestCase assert_equal 6, op.first[3], 'msync called with incorrect flags' end + def test_odd_size_discs_are_truncated_to_nearest_512 + # This should get rounded down to 1024 + @env.blocksize = 1024 + 511 + @env.writefile1('0') + @env.serve1 + client = FlexNBD::FakeSource.new(@env.ip, @env.port1, 'Connecting to server failed') + begin + result = client.read_hello + assert_equal 'NBDMAGIC', result[:passwd] + assert_equal 0x00420281861253, result[:magic] + assert_equal 1024, result[:size] + client.close + end + end end From 23d9ff587e9765dd19a1059ba9c14f1d2088ca0d Mon Sep 17 00:00:00 2001 From: Patrick J Cherry Date: Thu, 8 Feb 2018 16:36:20 +0000 Subject: [PATCH 2/3] Updated changelog --- debian/changelog | 1 + 1 file changed, 1 insertion(+) diff --git a/debian/changelog b/debian/changelog index f25f071..c29ecde 100644 --- a/debian/changelog +++ b/debian/changelog @@ -14,6 +14,7 @@ flexnbd (0.2.0) UNRELEASED; urgency=medium filesystem, and EINVAL to unknown commands. (#36, !40) * Proxy passes all NBD protocol errors through to the client instead of disconnecting and retrying (#36, !40) + * Ignore ends of discs that stray outside of 512-byte sector sizes (!42). -- James Carter Thu, 11 Jan 2018 10:05:35 +0000 From a19267b377d2fe435d2e34b2a08bd7c9e390f830 Mon Sep 17 00:00:00 2001 From: Patrick J Cherry Date: Thu, 8 Feb 2018 16:37:36 +0000 Subject: [PATCH 3/3] Adjust block-rounding line to match in serve.c --- src/common/ioutil.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/ioutil.c b/src/common/ioutil.c index 1926e86..dfa47df 100644 --- a/src/common/ioutil.c +++ b/src/common/ioutil.c @@ -103,7 +103,7 @@ int open_and_mmap(const char* filename, int* out_fd, uint64_t *out_size, void ** */ if ( size != (size & (~0x1ff))) { warn("file does not fit into 512-byte sectors; the end of the file will be ignored."); - size = size & (~0x1ff); + size &= ~0x1ff; } if (out_size) {