From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp50.i.mail.ru (smtp50.i.mail.ru [94.100.177.110]) (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 72A174696C4 for ; Fri, 17 Apr 2020 23:16:48 +0300 (MSK) From: Nikita Pettik Date: Fri, 17 Apr 2020 23:16:45 +0300 Message-Id: <878773d43bdc79130168b0beaf4e20aed795ddad.1587154034.git.korablev@tarantool.org> In-Reply-To: References: In-Reply-To: References: Subject: [Tarantool-patches] [PATCH v2 1/2] box/error: don't allow overflow of error ref counter List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org Cc: v.shpilevoy@tarantool.org There's no overflow check while incrementing error's reference counter in error_ref(). Meanwhile, stubborn users still may achieve overflow: each call of box.error.last() increments reference counter of error residing in diagnostic area. As a result, 2^32 calls of box.error.last() in a row will lead to counter overflow ergo - to unpredictable results. Let's fix it and introduce dummy check in error_ref(). --- src/lib/core/diag.c | 1 + src/lib/core/diag.h | 8 +++++++- src/lua/error.c | 3 ++- src/lua/error.lua | 4 ++++ 4 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/lib/core/diag.c b/src/lib/core/diag.c index e143db18d..5ddf543cb 100644 --- a/src/lib/core/diag.c +++ b/src/lib/core/diag.c @@ -60,6 +60,7 @@ error_set_prev(struct error *e, struct error *prev) */ error_unlink_effect(prev); } + assert(prev->refs < INT32_MAX); error_ref(prev); prev->effect = e; } diff --git a/src/lib/core/diag.h b/src/lib/core/diag.h index 7a5454d1c..fe852e640 100644 --- a/src/lib/core/diag.h +++ b/src/lib/core/diag.h @@ -112,10 +112,14 @@ struct error { struct error *effect; }; -static inline void +static inline int error_ref(struct error *e) { + assert(e->refs >= 0); + if (e->refs >= INT32_MAX) + return -1; e->refs++; + return 0; } static inline void @@ -247,6 +251,7 @@ static inline void diag_set_error(struct diag *diag, struct error *e) { assert(e != NULL); + assert(e->refs < INT32_MAX); error_ref(e); diag_clear(diag); error_unlink_effect(e); @@ -271,6 +276,7 @@ diag_add_error(struct diag *diag, struct error *e) */ assert(e->effect == NULL); assert(diag->last->effect == NULL); + assert(e->refs < INT32_MAX); error_ref(e); e->cause = diag->last; diag->last->effect = e; diff --git a/src/lua/error.c b/src/lua/error.c index 18a990a88..5c4af79e7 100644 --- a/src/lua/error.c +++ b/src/lua/error.c @@ -84,7 +84,8 @@ luaT_pusherror(struct lua_State *L, struct error *e) * It also important to reference the error first and only * then set the finalizer. */ - error_ref(e); + if (error_ref(e) != 0) + luaL_error(L, "Too many references to error object"); assert(CTID_CONST_STRUCT_ERROR_REF != 0); struct error **ptr = (struct error **) luaL_pushcdata(L, CTID_CONST_STRUCT_ERROR_REF); diff --git a/src/lua/error.lua b/src/lua/error.lua index bdc9c714d..7a2435b0a 100644 --- a/src/lua/error.lua +++ b/src/lua/error.lua @@ -118,6 +118,10 @@ local function error_set_prev(err, prev) if not ffi.istype('struct error', prev) and prev ~= nil then error("Usage: error1:set_prev(error2)") end + local INT32_MAX = 2147483647 + if err._refs >= INT32_MAX then + error("Too many references to error object") + end local ok = ffi.C.error_set_prev(err, prev); if ok ~= 0 then error("Cycles are not allowed") -- 2.17.1