From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 49188EC28C1; Tue, 4 Feb 2025 13:20:58 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 49188EC28C1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1738664458; bh=AlxyHnzoL5TZrrQgEeHSDeVFbZSIPoM9Q72O0PuSGRE=; h=Date:To:Cc:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=G/SFktKdqpMkFysFphP678Mn9GsGj6LVvX5PwfVTKImTmz8SDFvUaUyJe9EVRaYgn UCURyBYTnML3DfY/HhBk9JqMf0jTdcz0Q38Ynf9goJxEBBP+0rmiBItdV1pMhh271t FYmWNlxzXBsAOzuXOyE7OOi+Yn60shgzSuhn06uk= Received: from send57.i.mail.ru (send57.i.mail.ru [89.221.237.152]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 791EEEC28C7 for ; Tue, 4 Feb 2025 13:20:56 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 791EEEC28C7 Received: by exim-smtp-6d97ff8cf4-bk5jr with esmtpa (envelope-from ) id 1tfG35-00000000P44-17ct; Tue, 04 Feb 2025 13:20:55 +0300 Content-Type: multipart/alternative; boundary="------------bejVq5fmtMy0hJs0k2dCLtES" Message-ID: Date: Tue, 4 Feb 2025 13:20:55 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Sergey Kaplun Cc: tarantool-patches@dev.tarantool.org References: <20250203133122.3617-1-skaplun@tarantool.org> Content-Language: en-US In-Reply-To: <20250203133122.3617-1-skaplun@tarantool.org> X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: EEAE043A70213CC8 X-77F55803: 4F1203BC0FB41BD9279F651DE5037A5080B62BE5EE696263B6D179FE0FDCC17E1313CFAB8367EF908E2BE116634AD74D0B21648C9B88FAC406104296DCCA83D2AED2310DC27E867002352DEFE1562E43FB652AD2CEFA51F0 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7CC84CC3AD347B910EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637EEA194BB48C104EF8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8BDCB57F1115289963511383B46282F205FF8E1678D08FCE5CC7F00164DA146DAFE8445B8C89999728AA50765F7900637BA939FD1B3BAB99B389733CBF5DBD5E9C8A9BA7A39EFB766F5D81C698A659EA7CC7F00164DA146DA9985D098DBDEAEC80CABCCA60F52D7EBF6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA73AA81AA40904B5D9A18204E546F3947C2D01283D1ACF37BAC0837EA9F3D197644AD6D5ED66289B523666184CF4C3C14F6136E347CC761E07725E5C173C3A84C36E36DCD5FF651F90BA3038C0950A5D36B5C8C57E37DE458B330BD67F2E7D9AF16D1867E19FE14079C09775C1D3CA48CF3D321E7403792E342EB15956EA79C166A417C69337E82CC275ECD9A6C639B01B78DA827A17800CE7CD707F342D9BDC98731C566533BA786AA5CC5B56E945C8DA X-C1DE0DAB: 0D63561A33F958A5346FB6E5E71AE2865002B1117B3ED6966C9E6DFAC7644DB219AC5B239BAD4335823CB91A9FED034534781492E4B8EEAD37F46C620FF2CAEEBDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CFD71125997B23BE398CB895BC3CD868DD8E31952B741C40094F1751CC430F824CB94BA8791A304480C56FE0738BD31C044F034C02B8C4C5268C88C2EAC4B811AD31A0CC29C3192A5E111DC66A97D0BFE2913E6812662D5F2AB9AF64DB4688768036DF5FE9C0001AF333F2C28C22F508233FCF178C6DD14203 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojks+hT+CyYL3ABzJ80B+c3w== X-Mailru-Sender: 520A125C2F17F0B1E52FEF5D219D61400B21648C9B88FAC406104296DCCA83D20C639D352377E65E0152A3D17938EB451EB5A0BCEC6A560B3DDE9B364B0DF289BE2DA36745F2EEB5CEBA01FB949A1F1EEAB4BC95F72C04283CDA0F3B3F5B9367 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit] Reject negative getfenv()/setfenv() levels to prevent compiler warning. X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Bronnikov via Tarantool-patches Reply-To: Sergey Bronnikov Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" This is a multi-part message in MIME format. --------------bejVq5fmtMy0hJs0k2dCLtES Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hello, Sergey! thanks for the patch! LGTM Sergey On 03.02.2025 16:31, Sergey Kaplun wrote: > From: Mike Pall > > Thanks to Sergey Kaplun. > > (cherry picked from commit 9d777346bc4e3b033dd78393980d41ee7bc34867) > > When the number represented the level value is given to the > `getfenv()`/`setfenv()`, it is cast to the `int`. Assume the given value > is `2^31`, i.e. the resulting value after the cast is `INT_MIN`. After > this, it will be decremented in `lj_debug_level()` and underflowed to > the `INT_MAX`. That produces the UBSan warning about signed integer > overflow. > > This patch raises the error early in the aforementioned functions, since > a negative level value is meaningless. > > Sergey Kaplun: > * added the description and the test for the problem > > Part of tarantool/tarantool#11055 > --- > > Branch:https://github.com/tarantool/luajit/tree/skaplun/lj-1329-getfenv-setfenv-negative > Related issues: > *https://github.com/tarantool/tarantool/issues/11055 > *https://github.com/LuaJIT/LuaJIT/issues/1329 > > src/lib_base.c | 4 +++ > .../lj-1329-getfenv-setfenv-negative.test.lua | 27 +++++++++++++++++++ > 2 files changed, 31 insertions(+) > create mode 100644 test/tarantool-tests/lj-1329-getfenv-setfenv-negative.test.lua > > diff --git a/src/lib_base.c b/src/lib_base.c > index ad151975..23728d6c 100644 > --- a/src/lib_base.c > +++ b/src/lib_base.c > @@ -144,6 +144,8 @@ LJLIB_CF(getfenv) LJLIB_REC(.) > cTValue *o = L->base; > if (!(o < L->top && tvisfunc(o))) { > int level = lj_lib_optint(L, 1, 1); > + if (level < 0) > + lj_err_arg(L, 1, LJ_ERR_INVLVL); > o = lj_debug_frame(L, level, &level); > if (o == NULL) > lj_err_arg(L, 1, LJ_ERR_INVLVL); > @@ -166,6 +168,8 @@ LJLIB_CF(setfenv) > setgcref(L->env, obj2gco(t)); > return 0; > } > + if (level < 0) > + lj_err_arg(L, 1, LJ_ERR_INVLVL); > o = lj_debug_frame(L, level, &level); > if (o == NULL) > lj_err_arg(L, 1, LJ_ERR_INVLVL); > diff --git a/test/tarantool-tests/lj-1329-getfenv-setfenv-negative.test.lua b/test/tarantool-tests/lj-1329-getfenv-setfenv-negative.test.lua > new file mode 100644 > index 00000000..bc10c16f > --- /dev/null > +++ b/test/tarantool-tests/lj-1329-getfenv-setfenv-negative.test.lua > @@ -0,0 +1,27 @@ > +local tap = require('tap') > + > +-- The test file to demonstrate UBSan warning for `setfenv()` and > +-- `getfenv()` with a huge `level` value. > +-- See also:https://github.com/LuaJIT/LuaJIT/issues/1329. > +local test = tap.test('lj-1329-getfenv-setfenv-negative') > + > +test:plan(4) > + > +-- This number will be equal to `INT_MIN` when casted to `int`. > +-- After this, it will be decremented in `lj_debug_level()` and > +-- underflowed to the `INT_MAX`. That produces the UBSan warning > +-- about signed integer overflow. > +local LEVEL = 2 ^ 31 > +local ERRMSG = 'invalid level' > + > +-- Tests check the UBSan runtime error. Add assertions just to be > +-- sure that we don't change the behaviour. > +local status, errmsg = pcall(getfenv, LEVEL) > +test:ok(not status, 'getfenv: correct status') > +test:like(errmsg, ERRMSG, 'getfenv: correct error message') > + > +status, errmsg = pcall(setfenv, LEVEL, {}) > +test:ok(not status, 'setfenv: correct status') > +test:like(errmsg, ERRMSG, 'setfenv: correct error message') > + > +test:done(true) --------------bejVq5fmtMy0hJs0k2dCLtES Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 7bit

Hello, Sergey!

thanks for the patch! LGTM

Sergey

On 03.02.2025 16:31, Sergey Kaplun wrote:
From: Mike Pall <mike>

Thanks to Sergey Kaplun.

(cherry picked from commit 9d777346bc4e3b033dd78393980d41ee7bc34867)

When the number represented the level value is given to the
`getfenv()`/`setfenv()`, it is cast to the `int`. Assume the given value
is `2^31`, i.e. the resulting value after the cast is `INT_MIN`. After
this, it will be decremented in `lj_debug_level()` and underflowed to
the `INT_MAX`. That produces the UBSan warning about signed integer
overflow.

This patch raises the error early in the aforementioned functions, since
a negative level value is meaningless.

Sergey Kaplun:
* added the description and the test for the problem

Part of tarantool/tarantool#11055
---

Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-1329-getfenv-setfenv-negative
Related issues:
* https://github.com/tarantool/tarantool/issues/11055
* https://github.com/LuaJIT/LuaJIT/issues/1329

 src/lib_base.c                                |  4 +++
 .../lj-1329-getfenv-setfenv-negative.test.lua | 27 +++++++++++++++++++
 2 files changed, 31 insertions(+)
 create mode 100644 test/tarantool-tests/lj-1329-getfenv-setfenv-negative.test.lua

diff --git a/src/lib_base.c b/src/lib_base.c
index ad151975..23728d6c 100644
--- a/src/lib_base.c
+++ b/src/lib_base.c
@@ -144,6 +144,8 @@ LJLIB_CF(getfenv)		LJLIB_REC(.)
   cTValue *o = L->base;
   if (!(o < L->top && tvisfunc(o))) {
     int level = lj_lib_optint(L, 1, 1);
+    if (level < 0)
+      lj_err_arg(L, 1, LJ_ERR_INVLVL);
     o = lj_debug_frame(L, level, &level);
     if (o == NULL)
       lj_err_arg(L, 1, LJ_ERR_INVLVL);
@@ -166,6 +168,8 @@ LJLIB_CF(setfenv)
       setgcref(L->env, obj2gco(t));
       return 0;
     }
+    if (level < 0)
+      lj_err_arg(L, 1, LJ_ERR_INVLVL);
     o = lj_debug_frame(L, level, &level);
     if (o == NULL)
       lj_err_arg(L, 1, LJ_ERR_INVLVL);
diff --git a/test/tarantool-tests/lj-1329-getfenv-setfenv-negative.test.lua b/test/tarantool-tests/lj-1329-getfenv-setfenv-negative.test.lua
new file mode 100644
index 00000000..bc10c16f
--- /dev/null
+++ b/test/tarantool-tests/lj-1329-getfenv-setfenv-negative.test.lua
@@ -0,0 +1,27 @@
+local tap = require('tap')
+
+-- The test file to demonstrate UBSan warning for `setfenv()` and
+-- `getfenv()` with a huge `level` value.
+-- See also: https://github.com/LuaJIT/LuaJIT/issues/1329.
+local test = tap.test('lj-1329-getfenv-setfenv-negative')
+
+test:plan(4)
+
+-- This number will be equal to `INT_MIN` when casted to `int`.
+-- After this, it will be decremented in `lj_debug_level()` and
+-- underflowed to the `INT_MAX`. That produces the UBSan warning
+-- about signed integer overflow.
+local LEVEL = 2 ^ 31
+local ERRMSG = 'invalid level'
+
+-- Tests check the UBSan runtime error. Add assertions just to be
+-- sure that we don't change the behaviour.
+local status, errmsg = pcall(getfenv, LEVEL)
+test:ok(not status, 'getfenv: correct status')
+test:like(errmsg, ERRMSG, 'getfenv: correct error message')
+
+status, errmsg = pcall(setfenv, LEVEL, {})
+test:ok(not status, 'setfenv: correct status')
+test:like(errmsg, ERRMSG, 'setfenv: correct error message')
+
+test:done(true)
--------------bejVq5fmtMy0hJs0k2dCLtES--