From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp5.mail.ru (smtp5.mail.ru [94.100.179.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 02412469719 for ; Thu, 5 Mar 2020 03:14:23 +0300 (MSK) From: Vladislav Shpilevoy Date: Thu, 5 Mar 2020 01:14:20 +0100 Message-Id: In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH 2/2] box: on cfg properly check memory quota List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org, korablev@tarantool.org, imun@tarantool.org box_check_config() didn't check memtx_memory and vinyl_memory upper bound. As a result, it was possible to set memory size higher than what the quota allows as maximum. That worked only when box.cfg() was called first time, because quota_init() does not check its value. Subsequent box.cfg() calls use quota_set(), which aborts the program if a size is too big. Only in debug mode. In release quota_set() also worked with any sizes. Closes #4705 --- src/box/box.cc | 46 +++++++++++++++++++++---------------------- test/box/cfg.result | 19 ++++++++++++++++-- test/box/cfg.test.lua | 5 +++++ 3 files changed, 44 insertions(+), 26 deletions(-) diff --git a/src/box/box.cc b/src/box/box.cc index 0212f34ad..9045cefe4 100644 --- a/src/box/box.cc +++ b/src/box/box.cc @@ -577,24 +577,16 @@ box_check_wal_max_size(int64_t wal_max_size) return wal_max_size; } -static int64_t -box_check_memtx_memory(int64_t memory) -{ - if (memory < 0) { - tnt_raise(ClientError, ER_CFG, "memtx_memory", - "must not be less than 0"); - } - return memory; -} - -static int64_t -box_check_vinyl_memory(int64_t memory) -{ - if (memory < 0) { - tnt_raise(ClientError, ER_CFG, "vinyl_memory", - "must not be less than 0"); - } - return memory; +static ssize_t +box_check_memory_quota(const char *quota_name) +{ + int64_t size = cfg_geti64(quota_name); + if (size >= 0 && (size_t) size <= QUOTA_MAX) + return size; + diag_set(ClientError, ER_CFG, quota_name, + tt_sprintf("must be >= 0 and <= %zu, but it is %lld", + QUOTA_MAX, size)); + return -1; } static void @@ -608,7 +600,8 @@ box_check_vinyl_options(void) double run_size_ratio = cfg_getd("vinyl_run_size_ratio"); double bloom_fpr = cfg_getd("vinyl_bloom_fpr"); - box_check_vinyl_memory(cfg_geti64("vinyl_memory")); + if (box_check_memory_quota("vinyl_memory") < 0) + diag_raise(); if (read_threads < 1) { tnt_raise(ClientError, ER_CFG, "vinyl_read_threads", @@ -666,7 +659,8 @@ box_check_config() box_check_checkpoint_count(cfg_geti("checkpoint_count")); box_check_wal_max_size(cfg_geti64("wal_max_size")); box_check_wal_mode(cfg_gets("wal_mode")); - box_check_memtx_memory(cfg_geti64("memtx_memory")); + if (box_check_memory_quota("memtx_memory") < 0) + diag_raise(); box_check_memtx_min_tuple_size(cfg_geti64("memtx_min_tuple_size")); box_check_vinyl_options(); if (box_check_sql_cache_size(cfg_geti("sql_cache_size")) != 0) @@ -895,8 +889,10 @@ box_set_memtx_memory(void) struct memtx_engine *memtx; memtx = (struct memtx_engine *)engine_by_name("memtx"); assert(memtx != NULL); - memtx_engine_set_memory_xc(memtx, - box_check_memtx_memory(cfg_geti64("memtx_memory"))); + ssize_t size = box_check_memory_quota("memtx_memory"); + if (size < 0) + diag_raise(); + memtx_engine_set_memory_xc(memtx, size); } void @@ -954,8 +950,10 @@ box_set_vinyl_memory(void) { struct engine *vinyl = engine_by_name("vinyl"); assert(vinyl != NULL); - vinyl_engine_set_memory_xc(vinyl, - box_check_vinyl_memory(cfg_geti64("vinyl_memory"))); + ssize_t size = box_check_memory_quota("vinyl_memory"); + if (size < 0) + diag_raise(); + vinyl_engine_set_memory_xc(vinyl, size); } void diff --git a/test/box/cfg.result b/test/box/cfg.result index 8024b5516..9dd417c4f 100644 --- a/test/box/cfg.result +++ b/test/box/cfg.result @@ -254,11 +254,13 @@ box.cfg{memtx_memory = "100500"} | ... box.cfg{memtx_memory = -1} | --- - | - error: 'Incorrect value for option ''memtx_memory'': must not be less than 0' + | - error: 'Incorrect value for option ''memtx_memory'': must be >= 0 and <= 4398046510080, + | but it is -1' | ... box.cfg{vinyl_memory = -1} | --- - | - error: 'Incorrect value for option ''vinyl_memory'': must not be less than 0' + | - error: 'Incorrect value for option ''vinyl_memory'': must be >= 0 and <= 4398046510080, + | but it is -1' | ... box.cfg{vinyl = "vinyl"} | --- @@ -268,6 +270,19 @@ box.cfg{vinyl_write_threads = "threads"} | --- | - error: 'Incorrect value for option ''vinyl_write_threads'': should be of type number' | ... +-- +-- gh-4705: too big memory size led to an assertion. +-- +box.cfg{memtx_memory = 5000000000000} + | --- + | - error: 'Incorrect value for option ''memtx_memory'': must be >= 0 and <= 4398046510080, + | but it is 5000000000000' + | ... +box.cfg{vinyl_memory = 5000000000000} + | --- + | - error: 'Incorrect value for option ''vinyl_memory'': must be >= 0 and <= 4398046510080, + | but it is 5000000000000' + | ... -------------------------------------------------------------------------------- -- Dynamic configuration check diff --git a/test/box/cfg.test.lua b/test/box/cfg.test.lua index e6a90d770..875466a25 100644 --- a/test/box/cfg.test.lua +++ b/test/box/cfg.test.lua @@ -28,6 +28,11 @@ box.cfg{memtx_memory = -1} box.cfg{vinyl_memory = -1} box.cfg{vinyl = "vinyl"} box.cfg{vinyl_write_threads = "threads"} +-- +-- gh-4705: too big memory size led to an assertion. +-- +box.cfg{memtx_memory = 5000000000000} +box.cfg{vinyl_memory = 5000000000000} -------------------------------------------------------------------------------- -- Dynamic configuration check -- 2.21.1 (Apple Git-122.3)