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 C605CD5442B; Wed, 4 Sep 2024 16:43:05 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org C605CD5442B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1725457385; bh=sorWsVV9xAhR82CUZ1vTS/a4JWstkTW1mcUvkhor9A0=; 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=Z5d+gZyeNcHj+DIvj+0iFQ0jOiMAl3t8RB2btAkEwt9zuyKYcSBpoKkt3AeIZ+/++ DWLhlEVWBM8kvblw3GuV3hHDBgnNyS4jbNixsVY2NUJz7rwpMbq6JTulcaT1rf6MJD ryo6a8ao0NyUZm/AEWCJRxu/xDMq5MdfDVQ7YYqY= Received: from smtp50.i.mail.ru (smtp50.i.mail.ru [95.163.41.92]) (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 56B27D54401 for ; Wed, 4 Sep 2024 16:43:04 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 56B27D54401 Received: by smtp50.i.mail.ru with esmtpa (envelope-from ) id 1slqHn-00000005Kq1-0NYb; Wed, 04 Sep 2024 16:43:03 +0300 Content-Type: multipart/alternative; boundary="------------9Hn94Iay41ddV0DNaVhDSi0D" Message-ID: <5af61bfe-8827-40b4-b69c-17b5dd67a58f@tarantool.org> Date: Wed, 4 Sep 2024 16:43:02 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Sergey Kaplun , Maxim Kokryashkin Cc: tarantool-patches@dev.tarantool.org References: <20240826102520.29541-1-skaplun@tarantool.org> Content-Language: en-US In-Reply-To: <20240826102520.29541-1-skaplun@tarantool.org> X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD978F4AA4899E1B8B5D1E664F26A84173CF1F29C6F894556B8182A05F538085040D29860B206299B87D27678DDAA8063148E7ABE2D6D88B49326F465F4A9B6420D56AF9A33750679FD X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7780BADBB53F1FA59C2099A533E45F2D0395957E7521B51C2CFCAF695D4D8E9FCEA1F7E6F0F101C6778DA827A17800CE724AEE3BBB50D470CEA1F7E6F0F101C6723150C8DA25C47586E58E00D9D99D84E1BDDB23E98D2D38B043BF0FB74779F36B42A897214526334416EFDDF5280CB7C13E06AFC5939C245A471835C12D1D9774AD6D5ED66289B5278DA827A17800CE71AE4D56B06699BBC9FA2833FD35BB23D2EF20D2F80756B5F868A13BD56FB6657A471835C12D1D977725E5C173C3A84C353FA85A707D24CADCC7F00164DA146DA6F5DAA56C3B73B237318B6A418E8EAB8D32BA5DBAC0009BE9E8FC8737B5C2249CF3C25C8DD2B34D876E601842F6C81A12EF20D2F80756B5FB606B96278B59C4276E601842F6C81A127C277FBC8AE2E8B466D8E7E0312D87C3AA81AA40904B5D99C9F4D5AE37F343AD1F44FA8B9022EA23BBE47FD9DD3FB595F5C1EE8F4F765FC8C7ADC89C2F0B2A5E2021AF6380DFAD18AA50765F79006378A4BC95AACA28A5322CA9DD8327EE4930A3850AC1BE2E735789C969B8F27C422C4224003CC83647689D4C264860C145E X-C1DE0DAB: 0D63561A33F958A505B9888FE403A9425002B1117B3ED696B8AFC728E16B37AF361FAC1196A180DE823CB91A9FED034534781492E4B8EEAD69BF13FED57427F1BDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CFA9650573AE49F6AE6C407677E1799C01A28B3FE51609E133A2F3054DE12701AF7622FCE73A124D833EE79E6E40FF6950E4F4198796050240DC33E912CC403281CD28438AEE3C85ED5F4332CA8FE04980913E6812662D5F2AB9AF64DB4688768036DF5FE9C0001AF333F2C28C22F508233FCF178C6DD14203 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojpeIfmLQPsUKftnraRHKW4w== X-Mailru-Sender: 520A125C2F17F0B1E52FEF5D219D61401D73F2F666C80611FC043686C24B6C82549E53BFD881A9630152A3D17938EB451EB5A0BCEC6A560B3DDE9B364B0DF289BE2DA36745F2EEB5CEBA01FB949A1F1EEAB4BC95F72C04283CDA0F3B3F5B9367 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit] Limit number of string format elements to compile. 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. --------------9Hn94Iay41ddV0DNaVhDSi0D Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hi, Sergey, thanks for the patch! See my comments below. Sergey On 26.08.2024 13:25, Sergey Kaplun wrote: > From: Mike Pall > > Reported by pwnhacker0x18. > > (cherry picked from commit 4fc48c50fe3f3f5a9680bada5c0c0d0d7eb345a3) > > When compiling `string.format()` with a huge sequence of elements, it is > possible that too many KGC IRs underflow the IR buffer. This patch > limits the maximum number of `string.format()` elements to be compiled > to 100. > > Sergey Kaplun: > * added the description and the test for the problem > > Part of tarantool/tarantool#10199 > --- > > Branch:https://github.com/tarantool/luajit/tree/skaplun/lj-1203-limit-format-elements > Related issues: > *https://github.com/tarantool/tarantool/issues/10199 > *https://github.com/LuaJIT/LuaJIT/issues/1203 > > src/lj_ffrecord.c | 2 ++ > .../lj-1203-limit-format-elements.test.lua | 28 +++++++++++++++++++ > 2 files changed, 30 insertions(+) > create mode 100644 test/tarantool-tests/lj-1203-limit-format-elements.test.lua > > diff --git a/src/lj_ffrecord.c b/src/lj_ffrecord.c > index d5fc081e..3b82d044 100644 > --- a/src/lj_ffrecord.c > +++ b/src/lj_ffrecord.c > @@ -962,6 +962,7 @@ static void LJ_FASTCALL recff_string_format(jit_State *J, RecordFFData *rd) > TRef hdr, tr; > FormatState fs; > SFormat sf; > + int nfmt = 0; > /* Specialize to the format string. */ > emitir(IRTG(IR_EQ, IRT_STR), trfmt, lj_ir_kstr(J, fmt)); > tr = hdr = recff_bufhdr(J); > @@ -1031,6 +1032,7 @@ static void LJ_FASTCALL recff_string_format(jit_State *J, RecordFFData *rd) > recff_nyiu(J, rd); > return; > } > + if (++nfmt > 100) lj_trace_err(J, LJ_TRERR_TRACEOV); > } > J->base[0] = emitir(IRT(IR_BUFSTR, IRT_STR), tr, hdr); > } > diff --git a/test/tarantool-tests/lj-1203-limit-format-elements.test.lua b/test/tarantool-tests/lj-1203-limit-format-elements.test.lua > new file mode 100644 > index 00000000..f17d4e37 > --- /dev/null > +++ b/test/tarantool-tests/lj-1203-limit-format-elements.test.lua > @@ -0,0 +1,28 @@ > +local tap = require('tap') > + > +-- Test file to demonstrate LuaJIT incorrect recording of > +-- `string.format()` function with huge amount of elements. > +-- See also:https://github.com/LuaJIT/LuaJIT/issues/1173. Seems a correct link is https://github.com/LuaJIT/LuaJIT/issues/1203 > + > +local test = tap.test('lj-1203-limit-format-elements'):skipcond({ > + ['Test requires JIT enabled'] = not jit.status(), > +}) > + > +test:plan(2) > + > +jit.opt.start('hotloop=1') > + > +-- XXX: Use a huge amount of format elements to process, which > +-- creates a lot of string constants. > +local NELEMENTS = 25000 Why 25000? It is reproduced with 10000 as well. > +local fmt = ('%'):rep(NELEMENTS * 2) > +local expected = ('%'):rep(NELEMENTS) > +local result > +for _ = 1, 4 do > + result = fmt:format() > +end > + > +test:ok(true, 'no IR buffer underflow') Why do you need this check? Why the following check it not enough? > +test:is(result, expected, 'correct result') > + > +test:done(true) --------------9Hn94Iay41ddV0DNaVhDSi0D Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 7bit

Hi, Sergey,

thanks for the patch!

See my comments below.

Sergey

On 26.08.2024 13:25, Sergey Kaplun wrote:
From: Mike Pall <mike>

Reported by pwnhacker0x18.

(cherry picked from commit 4fc48c50fe3f3f5a9680bada5c0c0d0d7eb345a3)

When compiling `string.format()` with a huge sequence of elements, it is
possible that too many KGC IRs underflow the IR buffer. This patch
limits the maximum number of `string.format()` elements to be compiled
to 100.

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

Part of tarantool/tarantool#10199
---

Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-1203-limit-format-elements
Related issues:
* https://github.com/tarantool/tarantool/issues/10199
* https://github.com/LuaJIT/LuaJIT/issues/1203

 src/lj_ffrecord.c                             |  2 ++
 .../lj-1203-limit-format-elements.test.lua    | 28 +++++++++++++++++++
 2 files changed, 30 insertions(+)
 create mode 100644 test/tarantool-tests/lj-1203-limit-format-elements.test.lua

diff --git a/src/lj_ffrecord.c b/src/lj_ffrecord.c
index d5fc081e..3b82d044 100644
--- a/src/lj_ffrecord.c
+++ b/src/lj_ffrecord.c
@@ -962,6 +962,7 @@ static void LJ_FASTCALL recff_string_format(jit_State *J, RecordFFData *rd)
   TRef hdr, tr;
   FormatState fs;
   SFormat sf;
+  int nfmt = 0;
   /* Specialize to the format string. */
   emitir(IRTG(IR_EQ, IRT_STR), trfmt, lj_ir_kstr(J, fmt));
   tr = hdr = recff_bufhdr(J);
@@ -1031,6 +1032,7 @@ static void LJ_FASTCALL recff_string_format(jit_State *J, RecordFFData *rd)
       recff_nyiu(J, rd);
       return;
     }
+    if (++nfmt > 100) lj_trace_err(J, LJ_TRERR_TRACEOV);
   }
   J->base[0] = emitir(IRT(IR_BUFSTR, IRT_STR), tr, hdr);
 }
diff --git a/test/tarantool-tests/lj-1203-limit-format-elements.test.lua b/test/tarantool-tests/lj-1203-limit-format-elements.test.lua
new file mode 100644
index 00000000..f17d4e37
--- /dev/null
+++ b/test/tarantool-tests/lj-1203-limit-format-elements.test.lua
@@ -0,0 +1,28 @@
+local tap = require('tap')
+
+-- Test file to demonstrate LuaJIT incorrect recording of
+-- `string.format()` function with huge amount of elements.
+-- See also: https://github.com/LuaJIT/LuaJIT/issues/1173.
Seems a correct link is https://github.com/LuaJIT/LuaJIT/issues/1203
+
+local test = tap.test('lj-1203-limit-format-elements'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
+
+test:plan(2)
+
+jit.opt.start('hotloop=1')
+
+-- XXX: Use a huge amount of format elements to process, which
+-- creates a lot of string constants.
+local NELEMENTS = 25000

Why 25000? It is reproduced with 10000 as well.


+local fmt = ('%'):rep(NELEMENTS * 2)
+local expected = ('%'):rep(NELEMENTS)
+local result
+for _ = 1, 4 do
+  result = fmt:format()
+end
+
+test:ok(true, 'no IR buffer underflow')
Why do you need this check? Why the following check it not enough?
+test:is(result, expected, 'correct result')
+
+test:done(true)
--------------9Hn94Iay41ddV0DNaVhDSi0D--