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 F128D5D11EA; Wed, 30 Aug 2023 13:53:24 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org F128D5D11EA DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1693392805; bh=LP+sRzKKzUyJ9BmGGtqGb0Kt/ltvT8VC+cRP9XCLoBw=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=K542xsvzdabMtfuf3lgGF72UEvxtjR1m2P7Teg/0Eu095rdLTUE12Mn4r486qNUq9 Xdqi19u0xjNUGfzR9fq8Y/gw3WcCZuVZCA+DrN+5NN3g6qTUvseYk0P1vk24kSYJQm VfNLu42hWFRFWyVp6uJD8m72c0uVdw+YpWGp2tuw= Received: from smtp58.i.mail.ru (smtp58.i.mail.ru [95.163.41.96]) (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 C0F895D11D7 for ; Wed, 30 Aug 2023 13:53:23 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org C0F895D11D7 Received: by smtp58.i.mail.ru with esmtpa (envelope-from ) id 1qbIp8-00DZds-2f; Wed, 30 Aug 2023 13:53:23 +0300 Date: Wed, 30 Aug 2023 13:53:22 +0300 To: Sergey Bronnikov Message-ID: <6cus2hpqotdwax3q66xznqgnkekikkhc6ndd47gyc76ki2v5qt@oyjw5ucvzhrd> References: <8b2d744f68eb138c2b2c37e1ac851181e303b485.1693305720.git.sergeyb@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <8b2d744f68eb138c2b2c37e1ac851181e303b485.1693305720.git.sergeyb@tarantool.org> X-Mailru-Src: smtp X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD96E142CFC92DB15CD54A6D482CE9F3536BD31E9EB83FC0CF7182A05F538085040947925C1D3008FD5A5C60831A60BEA5F57F9E1715B7FD976E9591FA5157E894E X-C1DE0DAB: 0D63561A33F958A57CBF5D2FEB2EB3DCB812CE966ACF2EA376CA2EF022DFA699F87CCE6106E1FC07E67D4AC08A07B9B0CF7CD7A0D5AA5F25CB5012B2E24CD356 X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF77DD89D51EBB7742DC8270968E61249B1004E42C50DC4CA955A7F0CF078B5EC49A30900B95165D341776B9FDE05FBA7A5E9D17779CF9B00BC5CAE9B8AA02A1DF4DD6F8D8903431FF38220F62AD1355C91D7E09C32AA3244CE591AF3B9D6A18A4B263F8302173E34E1DD47778AE04E04DBAD658CF5C8AB4025DA084F8E80FEBD396F07DFE06A4A8314E894E437E78228B66933FA05BD8EF0CAD958392AE682691 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojA5xtch+oMC6RDhkAUqCJWA== X-Mailru-Sender: 11C2EC085EDE56FA38FD4C59F7EFE40712B6FCE28E1CD8195BFDF53E027C13F4B7A24381E0E24C14D51284F0FE6F529ABC7555A253F5B200DF104D74F62EE79D27EC13EC74F6107F4198E0F3ECE9B5443453F38A29522196 X-Mras: OK Subject: Re: [Tarantool-patches] [PATCH luajit] Fix predict_next() in parser (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: Maxim Kokryashkin via Tarantool-patches Reply-To: Maxim Kokryashkin Cc: max.kokryashkin@gmail.com, tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi, Sergey! Thanks for the patch! LGTM, except for a few nits below. On Tue, Aug 29, 2023 at 01:42:40PM +0300, Sergey Bronnikov via Tarantool-patches wrote: > From: sergeyb@tarantool.org > > Reported by Sergey Bronnikov. #1054 > > (cherry picked from commit 309fb42b871b6414f53e0e0e708bce0b0d62daff) > > The following Lua snippet triggers an out of boundary access to a stack: > > ```lua > a, b, c = 1, 2, 3 > local d > for _ in nil do end > ``` > > With execution snippet by LuaJIT instrumented by ASAN it leads to > a heap-buffer-overflow. I suggest the following rephrasing with grammar fixes: | During the execution of this snippet with LuaJIT instrumented by ASAN, | it leads to a heap buffer overflow. > > In a function `predict_next` variable `exprpc` looks forward and expects Typo: s/In a/In/ > extra bytecodes on the stack. However, `KPRI` is merged to the `KNIL` Typo: s/to the/to/ > and there is no new bytecode to add, so `exprpc == fs->bclim` and it > leads to out of boundary access. The last sentence that you don't have here, but have on GitHub should look like the following: | The issue has been fixed by an early return when `pc >= fs->bclim`. > > Sergey Bronnikov: > * added the description and the test for the problem > > Part of tarantool/tarantool#8825 > --- > > PR: https://github.com/tarantool/tarantool/pull/9054 > Branch: https://github.com/tarantool/luajit/tree/ligurio/lj-1054-incorrect-pc-value-predict_next > Related issue: > * https://github.com/LuaJIT/LuaJIT/issues/1054 > > src/lj_parse.c | 4 +++- > ...incorrect-pc-value-in-predict_next.test.lua | 18 ++++++++++++++++++ > 2 files changed, 21 insertions(+), 1 deletion(-) > create mode 100644 test/tarantool-tests/lj-1054-incorrect-pc-value-in-predict_next.test.lua > > diff --git a/src/lj_parse.c b/src/lj_parse.c > index 343fa797..f1015960 100644 > --- a/src/lj_parse.c > +++ b/src/lj_parse.c > @@ -2511,9 +2511,11 @@ static void parse_for_num(LexState *ls, GCstr *varname, BCLine line) > */ > static int predict_next(LexState *ls, FuncState *fs, BCPos pc) > { > - BCIns ins = fs->bcbase[pc].ins; > + BCIns ins; > GCstr *name; > cTValue *o; > + if (pc >= fs->bclim) return 0; > + ins = fs->bcbase[pc].ins; > switch (bc_op(ins)) { > case BC_MOV: > name = gco2str(gcref(var_get(ls, fs, bc_d(ins)).name)); > diff --git a/test/tarantool-tests/lj-1054-incorrect-pc-value-in-predict_next.test.lua b/test/tarantool-tests/lj-1054-incorrect-pc-value-in-predict_next.test.lua > new file mode 100644 > index 00000000..17f1b994 > --- /dev/null > +++ b/test/tarantool-tests/lj-1054-incorrect-pc-value-in-predict_next.test.lua > @@ -0,0 +1,18 @@ > +local tap = require('tap') > +local test = tap.test('lj-1054-incorrect-pc-value-in-predict_next') > +test:plan(1) > + > + > +-- The test demonstrates a problem with out of boundary access to a stack. > +-- Sample executed in LuaJIT instrumented by ASAN leads to > +-- a heap-buffer-overflow. > +-- See also https://github.com/LuaJIT/LuaJIT/issues/528 This chunk is a bit dated and I don't really want to bother with going through a bunch of emails and sequential diffs, so I'll just bring the actual one here by myself. Here it is: -- The test demonstrates a problem with out-of-boundary -- access to a stack. The problem can be easily observed -- on execution the sample by LuaJIT by ASAN, sanitizer Typo: s/execution/execution of/ Typo: s/sanitizer/where the sanitizer/ -- reports a heap-based buffer overflow. -- See also https://github.com/LuaJIT/LuaJIT/issues/1054. Otherwise, considering the changes you've already made after Sergey's comments, this part is ok. > +local lua_code = [[ > +a, b, c = 1, 2, 3 > +local d > +for _ in nil do end > +]] > + > +test:ok(loadstring(lua_code), 'parsing is correct') > + > +test:done(true) > -- > 2.34.1 >