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 2933514E0D27; Mon, 8 Sep 2025 15:01:50 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 2933514E0D27 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1757332910; bh=9mzglk2OmEigkZnVwx8zXJON2iQctOaarED8WGjxQRc=; 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=hpeE75O5rpm0H7tScOPf4DXSJw2QixxUxOId5qMeni6/M7d5a25merHlOXlB28ro6 jExMFt564VjLLA+IYnsvjDJ3b+NC3BjU0fTlIc3LDY6+QCfc1nOICllCqe4sOqB7t3 xZA4yeu8bzogTtCGDuevm5P4XMSsunXit8iCge0w= Received: from send195.i.mail.ru (send195.i.mail.ru [95.163.59.34]) (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 ED3871B7EEE for ; Mon, 8 Sep 2025 15:01:48 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org ED3871B7EEE Received: by exim-smtp-c584fb9f-9krpf with esmtpa (envelope-from ) id 1uvaZA-00000000SZp-0Ddg; Mon, 08 Sep 2025 15:01:48 +0300 Content-Type: multipart/alternative; boundary="------------q53aG0Gq3rAWt9EJ9TogKxb0" Message-ID: <2879fd41-c26f-4dbf-851a-31748f8cff7e@tarantool.org> Date: Mon, 8 Sep 2025 15:01:47 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: Sergey Kaplun Cc: tarantool-patches@dev.tarantool.org References: <429b6cf7a260883bf337b13e30e0c64bc0a70723.1755844191.git.skaplun@tarantool.org> In-Reply-To: <429b6cf7a260883bf337b13e30e0c64bc0a70723.1755844191.git.skaplun@tarantool.org> X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: EEAE043A70213CC8 X-77F55803: 4F1203BC0FB41BD92FE000A65CB8EE9CB9E7CA6DB40EC00CFD5AD8708A9F8500182A05F5380850404C228DA9ACA6FE2788379BF6146D93303DE06ABAFEAF6705DD1E459F92E6E13AE0958443F9D6EC2A61707949AA19B90C X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7F65C230EDDCD559EEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637AC83A81C8FD4AD23D82A6BABE6F325AC2E85FA5F3EDFCBAA7353EFBB553375661343631359B0F9D9F8795A649C97C7565D2797B7AFAF1F83107E6FB5510098C0389733CBF5DBD5E913377AFFFEAFD269176DF2183F8FC7C0D9442B0B5983000E8941B15DA834481FCF19DD082D7633A0EF3E4896CB9E6436389733CBF5DBD5E9D5E8D9A59859A8B65FF0BFC5AEE34BE6CC7F00164DA146DA6F5DAA56C3B73B237318B6A418E8EAB8D32BA5DBAC0009BE9E8FC8737B5C22495276EF5FA9B1B5E376E601842F6C81A12EF20D2F80756B5FB606B96278B59C4276E601842F6C81A127C277FBC8AE2E8B4EC14D5F6B0DF2963AA81AA40904B5D99C9F4D5AE37F343AD1F44FA8B9022EA23BBE47FD9DD3FB595F5C1EE8F4F765FC72CEEB2601E22B093A03B725D353964B0B7D0EA88DDEDAC722CA9DD8327EE4930A3850AC1BE2E735B7122A4C44A4E42FC4224003CC83647689D4C264860C145E X-C1DE0DAB: 0D63561A33F958A53DF6F2C4D72FDD605002B1117B3ED696321968E049DF3664466072E6821086B3823CB91A9FED034534781492E4B8EEAD2B25D9E4C92BC8ACBDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CF8F3A19392EF3B9C78975E8178451FB56B1BC446E32CABAC59202B0DA8C74A28CE4A5F4FEF661E8B51E832756A85E66F7487BC7D781300AF56E014C93A7FB94F11C7DE997021F034C111DC66A97D0BFE2913E6812662D5F2AB9AF64DB4688768036DF5FE9C0001AF333F2C28C22F508233FCF178C6DD14203 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu53w8ahmwBjZKM/YPHZyZHvz5uv+WouB9+ObcCpyrx6l7KImUglyhkEat/+ysWwi0gdhEs0JGjl6ggRWTy1haxBpVdbIX1nthFXMZebaIdHP2ghjoIc/363UZI6Kf1ptIMVdVMtzNxwZu5Wuifxe/ku44= X-Mailru-Sender: 811C44EDE0507D1F797560C68D020EBD13B85D1E9EADB67E72C88FB39F7EDAEDFC1CC0064D8D9A84524C3F4074963667645D15D82EE4B272BD6E4642A116CA93524AA66B5ACBE6721EF430B9A63E2A504198E0F3ECE9B5443453F38A29522196 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit 2/2] FFI: Fix dangling CType references (again). 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. --------------q53aG0Gq3rAWt9EJ9TogKxb0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hi, Sergey, thanks for the patch! LGTM Sergey On 8/22/25 09:36, Sergey Kaplun wrote: > From: Mike Pall > > Reported by Sergey Kaplun. > > (cherry picked from commit c64020f3c6d124503213147f2fb47c20335a395b) > > This patch fixes JIT recording for the vararg calls, that was not fixed > in the previous patch. The `ctr` pointer to the child cdata value may > become invalid after the ctype state table reallocation in the > `crec_call_args()`. > > This patch preserves `ctr->info` in the separate variable to avoid > dereferencing an invalid pointer. > > Sergey Kaplun: > * added the description and the test for the problem > > Part of tarantool/tarantool#11691 > --- > src/lj_crecord.c | 11 +-- > ...0-dangling-ctype-ref-on-ccall-jit.test.lua | 82 +++++++++++++++++++ > 2 files changed, 88 insertions(+), 5 deletions(-) > create mode 100644 test/tarantool-tests/lj-1360-dangling-ctype-ref-on-ccall-jit.test.lua > > diff --git a/src/lj_crecord.c b/src/lj_crecord.c > index 935106dc..b016eaec 100644 > --- a/src/lj_crecord.c > +++ b/src/lj_crecord.c > @@ -1236,6 +1236,7 @@ static int crec_call(jit_State *J, RecordFFData *rd, GCcdata *cd) > if (ctype_isfunc(info)) { > TRef func = emitir(IRT(IR_FLOAD, tp), J->base[0], IRFL_CDATA_PTR); > CType *ctr = ctype_rawchild(cts, ct); > + CTInfo ctr_info = ctr->info; /* crec_call_args may invalidate ctr. */ > IRType t = crec_ct2irt(cts, ctr); > TRef tr; > TValue tv; > @@ -1243,11 +1244,11 @@ static int crec_call(jit_State *J, RecordFFData *rd, GCcdata *cd) > tv.u64 = ((uintptr_t)cdata_getptr(cdataptr(cd), (LJ_64 && tp == IRT_P64) ? 8 : 4) >> 2) | U64x(800000000, 00000000); > if (tvistrue(lj_tab_get(J->L, cts->miscmap, &tv))) > lj_trace_err(J, LJ_TRERR_BLACKL); > - if (ctype_isvoid(ctr->info)) { > + if (ctype_isvoid(ctr_info)) { > t = IRT_NIL; > rd->nres = 0; > - } else if (!(ctype_isnum(ctr->info) || ctype_isptr(ctr->info) || > - ctype_isenum(ctr->info)) || t == IRT_CDATA) { > + } else if (!(ctype_isnum(ctr_info) || ctype_isptr(ctr_info) || > + ctype_isenum(ctr_info)) || t == IRT_CDATA) { > lj_trace_err(J, LJ_TRERR_NYICALL); > } > if ((info & CTF_VARARG) > @@ -1258,7 +1259,7 @@ static int crec_call(jit_State *J, RecordFFData *rd, GCcdata *cd) > func = emitir(IRT(IR_CARG, IRT_NIL), func, > lj_ir_kint(J, ctype_typeid(cts, ct))); > tr = emitir(IRT(IR_CALLXS, t), crec_call_args(J, rd, cts, ct), func); > - if (ctype_isbool(ctr->info)) { > + if (ctype_isbool(ctr_info)) { > if (frame_islua(J->L->base-1) && bc_b(frame_pc(J->L->base-1)[-1]) == 1) { > /* Don't check result if ignored. */ > tr = TREF_NIL; > @@ -1274,7 +1275,7 @@ static int crec_call(jit_State *J, RecordFFData *rd, GCcdata *cd) > tr = TREF_TRUE; > } > } else if (t == IRT_PTR || (LJ_64 && t == IRT_P32) || > - t == IRT_I64 || t == IRT_U64 || ctype_isenum(ctr->info)) { > + t == IRT_I64 || t == IRT_U64 || ctype_isenum(ctr_info)) { > TRef trid = lj_ir_kint(J, ctype_cid(info)); > tr = emitir(IRTG(IR_CNEWI, IRT_CDATA), trid, tr); > if (t == IRT_I64 || t == IRT_U64) lj_needsplit(J); > diff --git a/test/tarantool-tests/lj-1360-dangling-ctype-ref-on-ccall-jit.test.lua b/test/tarantool-tests/lj-1360-dangling-ctype-ref-on-ccall-jit.test.lua > new file mode 100644 > index 00000000..388bec6e > --- /dev/null > +++ b/test/tarantool-tests/lj-1360-dangling-ctype-ref-on-ccall-jit.test.lua > @@ -0,0 +1,82 @@ > +local tap = require('tap') > +local ffi = require('ffi') > + > +-- This test demonstrates LuaJIT's incorrect behaviour when the > +-- reallocation of `cts->tab` strikes during the recording of the > +-- setup arguments for the FFI call. > +-- Before the patch, the test failed only under ASAN. > +-- This file tests the case similar to the > +-- , but for the > +-- compiler part only. > +-- See alsohttps://github.com/LuaJIT/LuaJIT/issues/1360. > +local test = tap.test('lj-1360-dangling-ctype-ref-on-ccall'):skipcond({ > + ['Impossible to predict the value of cts->top'] = _TARANTOOL, > + ['Test requires JIT enabled'] = not jit.status(), > +}) > + > +test:plan(1) > + > +-- XXX: Declare the structure to increase `cts->top` up to 128 > +-- slots. The setting up of function's arguments to the next call > +-- will reallocate the `cts->tab` during `lj_ccall_ctid_vararg()` > +-- and `ccall_set_args()` will use a dangling reference. > +ffi.cdef[[ > + struct test { > + int a; > + int b; > + int c; > + int d; > + int e; > + int f; > + int g; > + int h; > + int i; > + int j; > + int k; > + int l; > + int m; > + int n; > + int o; > + int p; > + int q; > + int r; > + int s; > + int t; > + int u; > + int v; > + int w; > + int x; > + int y; > + int z; > + int aa; > + int ab; > + int ac; > + int ad; > + }; > + // Use an existing function that actually takes no arguments. > + // We can declare it however we want. > + // Need a vararg function for this issue. > + int getppid(...); > +]] > + > +local arg_call = ffi.new('struct test') > + > +-- Place `getppid` symbol in cache to permit recording. > +ffi.C.getppid(0) > + > +-- Assertions to check the `cts->top` value. > +assert(ffi.typeinfo(127), 'cts->top >= 127') > +assert(not ffi.typeinfo(128), 'cts->top < 128') > + > +jit.opt.start('hotloop=1') > +local counter = 0 > +while counter < 4 do > + -- Don't check the result, just check that there is no invalid > + -- memory access. > + ffi.C.getppid(arg_call) > + counter = counter + 1 > +end > + > +test:ok(true, 'no heap-use-after-free in C call') > + > +test:done(true) --------------q53aG0Gq3rAWt9EJ9TogKxb0 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 7bit

Hi, Sergey,

thanks for the patch! LGTM

Sergey

On 8/22/25 09:36, Sergey Kaplun wrote:
From: Mike Pall <mike>

Reported by Sergey Kaplun.

(cherry picked from commit c64020f3c6d124503213147f2fb47c20335a395b)

This patch fixes JIT recording for the vararg calls, that was not fixed
in the previous patch. The `ctr` pointer to the child cdata value may
become invalid after the ctype state table reallocation in the
`crec_call_args()`.

This patch preserves `ctr->info` in the separate variable to avoid
dereferencing an invalid pointer.

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

Part of tarantool/tarantool#11691
---
 src/lj_crecord.c                              | 11 +--
 ...0-dangling-ctype-ref-on-ccall-jit.test.lua | 82 +++++++++++++++++++
 2 files changed, 88 insertions(+), 5 deletions(-)
 create mode 100644 test/tarantool-tests/lj-1360-dangling-ctype-ref-on-ccall-jit.test.lua

diff --git a/src/lj_crecord.c b/src/lj_crecord.c
index 935106dc..b016eaec 100644
--- a/src/lj_crecord.c
+++ b/src/lj_crecord.c
@@ -1236,6 +1236,7 @@ static int crec_call(jit_State *J, RecordFFData *rd, GCcdata *cd)
   if (ctype_isfunc(info)) {
     TRef func = emitir(IRT(IR_FLOAD, tp), J->base[0], IRFL_CDATA_PTR);
     CType *ctr = ctype_rawchild(cts, ct);
+    CTInfo ctr_info = ctr->info;  /* crec_call_args may invalidate ctr. */
     IRType t = crec_ct2irt(cts, ctr);
     TRef tr;
     TValue tv;
@@ -1243,11 +1244,11 @@ static int crec_call(jit_State *J, RecordFFData *rd, GCcdata *cd)
     tv.u64 = ((uintptr_t)cdata_getptr(cdataptr(cd), (LJ_64 && tp == IRT_P64) ? 8 : 4) >> 2) | U64x(800000000, 00000000);
     if (tvistrue(lj_tab_get(J->L, cts->miscmap, &tv)))
       lj_trace_err(J, LJ_TRERR_BLACKL);
-    if (ctype_isvoid(ctr->info)) {
+    if (ctype_isvoid(ctr_info)) {
       t = IRT_NIL;
       rd->nres = 0;
-    } else if (!(ctype_isnum(ctr->info) || ctype_isptr(ctr->info) ||
-		 ctype_isenum(ctr->info)) || t == IRT_CDATA) {
+    } else if (!(ctype_isnum(ctr_info) || ctype_isptr(ctr_info) ||
+		 ctype_isenum(ctr_info)) || t == IRT_CDATA) {
       lj_trace_err(J, LJ_TRERR_NYICALL);
     }
     if ((info & CTF_VARARG)
@@ -1258,7 +1259,7 @@ static int crec_call(jit_State *J, RecordFFData *rd, GCcdata *cd)
       func = emitir(IRT(IR_CARG, IRT_NIL), func,
 		    lj_ir_kint(J, ctype_typeid(cts, ct)));
     tr = emitir(IRT(IR_CALLXS, t), crec_call_args(J, rd, cts, ct), func);
-    if (ctype_isbool(ctr->info)) {
+    if (ctype_isbool(ctr_info)) {
       if (frame_islua(J->L->base-1) && bc_b(frame_pc(J->L->base-1)[-1]) == 1) {
 	/* Don't check result if ignored. */
 	tr = TREF_NIL;
@@ -1274,7 +1275,7 @@ static int crec_call(jit_State *J, RecordFFData *rd, GCcdata *cd)
 	tr = TREF_TRUE;
       }
     } else if (t == IRT_PTR || (LJ_64 && t == IRT_P32) ||
-	       t == IRT_I64 || t == IRT_U64 || ctype_isenum(ctr->info)) {
+	       t == IRT_I64 || t == IRT_U64 || ctype_isenum(ctr_info)) {
       TRef trid = lj_ir_kint(J, ctype_cid(info));
       tr = emitir(IRTG(IR_CNEWI, IRT_CDATA), trid, tr);
       if (t == IRT_I64 || t == IRT_U64) lj_needsplit(J);
diff --git a/test/tarantool-tests/lj-1360-dangling-ctype-ref-on-ccall-jit.test.lua b/test/tarantool-tests/lj-1360-dangling-ctype-ref-on-ccall-jit.test.lua
new file mode 100644
index 00000000..388bec6e
--- /dev/null
+++ b/test/tarantool-tests/lj-1360-dangling-ctype-ref-on-ccall-jit.test.lua
@@ -0,0 +1,82 @@
+local tap = require('tap')
+local ffi = require('ffi')
+
+-- This test demonstrates LuaJIT's incorrect behaviour when the
+-- reallocation of `cts->tab` strikes during the recording of the
+-- setup arguments for the FFI call.
+-- Before the patch, the test failed only under ASAN.
+-- This file tests the case similar to the
+-- <lj-1360-dangling-ctype-ref-on-ccall.test.lua>, but for the
+-- compiler part only.
+-- See also https://github.com/LuaJIT/LuaJIT/issues/1360.
+local test = tap.test('lj-1360-dangling-ctype-ref-on-ccall'):skipcond({
+  ['Impossible to predict the value of cts->top'] = _TARANTOOL,
+  ['Test requires JIT enabled'] = not jit.status(),
+})
+
+test:plan(1)
+
+-- XXX: Declare the structure to increase `cts->top` up to 128
+-- slots. The setting up of function's arguments to the next call
+-- will reallocate the `cts->tab` during `lj_ccall_ctid_vararg()`
+-- and `ccall_set_args()` will use a dangling reference.
+ffi.cdef[[
+  struct test {
+      int a;
+      int b;
+      int c;
+      int d;
+      int e;
+      int f;
+      int g;
+      int h;
+      int i;
+      int j;
+      int k;
+      int l;
+      int m;
+      int n;
+      int o;
+      int p;
+      int q;
+      int r;
+      int s;
+      int t;
+      int u;
+      int v;
+      int w;
+      int x;
+      int y;
+      int z;
+      int aa;
+      int ab;
+      int ac;
+      int ad;
+  };
+  // Use an existing function that actually takes no arguments.
+  // We can declare it however we want.
+  // Need a vararg function for this issue.
+  int getppid(...);
+]]
+
+local arg_call = ffi.new('struct test')
+
+-- Place `getppid` symbol in cache to permit recording.
+ffi.C.getppid(0)
+
+-- Assertions to check the `cts->top` value.
+assert(ffi.typeinfo(127), 'cts->top >= 127')
+assert(not ffi.typeinfo(128), 'cts->top < 128')
+
+jit.opt.start('hotloop=1')
+local counter = 0
+while counter < 4 do
+  -- Don't check the result, just check that there is no invalid
+  -- memory access.
+  ffi.C.getppid(arg_call)
+  counter = counter + 1
+end
+
+test:ok(true, 'no heap-use-after-free in C call')
+
+test:done(true)
--------------q53aG0Gq3rAWt9EJ9TogKxb0--