From 1c66b56af1b4961a4a7c765e546f438745ed2a66 Mon Sep 17 00:00:00 2001 From: Patrick J Cherry Date: Mon, 12 Feb 2018 19:04:29 +0000 Subject: [PATCH 1/4] Update proxy malloc to add the struct size onto the request/response buffer This alters the meaning of NBD_MAX_SIZE to be the actual max request size we'll accept over nbd. Previously it was *nearly* the max size we'd accept depending on the size of the struct. --- src/proxy/proxy.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/proxy/proxy.c b/src/proxy/proxy.c index b323c68..951542c 100644 --- a/src/proxy/proxy.c +++ b/src/proxy/proxy.c @@ -76,8 +76,12 @@ struct proxier* proxy_create( } out->init.buf = xmalloc( sizeof( struct nbd_init_raw ) ); - out->req.buf = xmalloc( NBD_MAX_SIZE ); - out->rsp.buf = xmalloc( NBD_MAX_SIZE ); + + /* Add on the request / response size to our malloc to accommodate both + * the struct and the data + */ + out->req.buf = xmalloc( NBD_MAX_SIZE + NBD_REQUEST_SIZE ); + out->rsp.buf = xmalloc( NBD_MAX_SIZE + NBD_RESPONSE_SIZE ); log_context = xmalloc( strlen(s_upstream_address) + strlen(s_upstream_port) + 2 ); sprintf(log_context, "%s:%s", s_upstream_address, s_upstream_port); @@ -452,15 +456,18 @@ int proxy_read_from_downstream( struct proxier *proxy, int state ) return EXIT; } - /* Simple validations */ + /* Simple validations -- the request / reply size have already + * been taken into account in the xmalloc, so no need to worry + * about them here + */ if ( request->type == REQUEST_READ ) { - if (request->len > ( NBD_MAX_SIZE - NBD_REPLY_SIZE ) ) { + if ( request->len > NBD_MAX_SIZE ) { warn( "NBD read request size %"PRIu32" too large", request->len ); return EXIT; } } if ( request->type == REQUEST_WRITE ) { - if (request->len > ( NBD_MAX_SIZE - NBD_REQUEST_SIZE ) ) { + if ( request->len > NBD_MAX_SIZE ) { warn( "NBD write request size %"PRIu32" too large", request->len ); return EXIT; } From 158379ba7a3f09a7855399a0e653a8a5c5f770a6 Mon Sep 17 00:00:00 2001 From: Patrick J Cherry Date: Mon, 12 Feb 2018 19:11:24 +0000 Subject: [PATCH 2/4] Use correct constant name. --- src/proxy/proxy.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/proxy/proxy.c b/src/proxy/proxy.c index 951542c..291acd6 100644 --- a/src/proxy/proxy.c +++ b/src/proxy/proxy.c @@ -77,11 +77,11 @@ struct proxier* proxy_create( out->init.buf = xmalloc( sizeof( struct nbd_init_raw ) ); - /* Add on the request / response size to our malloc to accommodate both + /* Add on the request / reply size to our malloc to accommodate both * the struct and the data */ out->req.buf = xmalloc( NBD_MAX_SIZE + NBD_REQUEST_SIZE ); - out->rsp.buf = xmalloc( NBD_MAX_SIZE + NBD_RESPONSE_SIZE ); + out->rsp.buf = xmalloc( NBD_MAX_SIZE + NBD_REPLY_SIZE ); log_context = xmalloc( strlen(s_upstream_address) + strlen(s_upstream_port) + 2 ); sprintf(log_context, "%s:%s", s_upstream_address, s_upstream_port); From bb1f6ecdf5eea5e59725f4e96d4635da03fbe5da Mon Sep 17 00:00:00 2001 From: Patrick J Cherry Date: Tue, 13 Feb 2018 15:51:09 +0000 Subject: [PATCH 3/4] Updated changelog --- debian/changelog | 2 ++ 1 file changed, 2 insertions(+) diff --git a/debian/changelog b/debian/changelog index c29ecde..5ef9dbb 100644 --- a/debian/changelog +++ b/debian/changelog @@ -15,6 +15,8 @@ flexnbd (0.2.0) UNRELEASED; urgency=medium * 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). + * Alter semantics of NBD_MAX_BLOCK_SIZE to remove struct size overheads when + calculating if a request exceeds the max block size (!45) -- James Carter Thu, 11 Jan 2018 10:05:35 +0000 From 2e17e8955fef0ecb4f2c23dcf02ba8ee27788088 Mon Sep 17 00:00:00 2001 From: Patrick J Cherry Date: Tue, 13 Feb 2018 17:04:51 +0000 Subject: [PATCH 4/4] Added tests for NBD_MAX_SIZE This constant is only used in the proxy, so the tests only cover proxy mode. --- tests/acceptance/proxy_tests.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/acceptance/proxy_tests.rb b/tests/acceptance/proxy_tests.rb index 65fe38a..897c487 100644 --- a/tests/acceptance/proxy_tests.rb +++ b/tests/acceptance/proxy_tests.rb @@ -207,4 +207,17 @@ module ProxyTests end end end + + def test_maximum_write_request_size + # Defined in src/common/nbdtypes.h + nbd_max_block_size = 32 * 1024 * 1024 + @env.writefile1('0' * 40 * 1024) + with_proxied_client do |client| + # This will crash with EPIPE if the proxy dies. + client.write(0, b * nbd_max_block_size) + rsp = client.read_response + assert_equal FlexNBD::REPLY_MAGIC, rsp[:magic] + assert_equal 0, rsp[:error] + end + end end