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 1D93355BD6E; Mon, 31 Jul 2023 15:01:11 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 1D93355BD6E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1690804871; bh=OVHbwVXpQzZoTX9BwmZPdg9N4ArY2sMILnb4p/8zhN8=; 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=BTDRBgAJ1b+ldOTDzb5Aq8rQYnCzlcNOptDfP1t46ivgpFgX2QPc+9xPw5RNBVW/V ddw/DeFwKEj6dotXF8xdyLt64Koxv8mSb4OZEo7MqZJ5EskuriIXHx62xM6xL8tR2X hvRsLLm8rGU/IvQTigo2gEoFXA/HIiLa+bAKIO5A= Received: from smtp47.i.mail.ru (smtp47.i.mail.ru [95.163.41.85]) (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 3BFCC55A1F1 for ; Mon, 31 Jul 2023 15:01:09 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 3BFCC55A1F1 Received: by smtp47.i.mail.ru with esmtpa (envelope-from ) id 1qQRaG-009ELg-JE; Mon, 31 Jul 2023 15:01:08 +0300 Date: Mon, 31 Jul 2023 15:01:08 +0300 To: Sergey Bronnikov Message-ID: <5t3vejxwlcpd3fczyenum4527mtwtmt6qd4agwrxhnkt3zdtob@62fmw5pdbhqg> References: <6426e58a9a72691ccffc84001c21e363c8da6312.1690300762.git.sergeyb@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <6426e58a9a72691ccffc84001c21e363c8da6312.1690300762.git.sergeyb@tarantool.org> X-Mailru-Src: smtp X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD920919EE866B1AD243D6E4888BA110515B2490C5D3E001F4E00894C459B0CD1B9A2F8C73AEF8CB824C8CA47019E7B9F0B162D3CB530D1A47CD8331305C3D564B6 X-C1DE0DAB: 0D63561A33F958A5FB3F46ECE855AED0147E3B94CFCB55014661BD3376334BA5F87CCE6106E1FC07E67D4AC08A07B9B0B355ED1E20F5346ACB5012B2E24CD356 X-C8649E89: 1C3962B70DF3F0AD5177F0B940C8B66ECE892A7B2722663E91682638B966EB3F662256BEEFA9527FF32515B26D27BD94CF7252036607C112AE5FDC49377F500F28F44CD05321E44CEB42B588A096AD8937FD76D11AF80A0D70BD4D0BE2F8B3CA9EEF11816AA1A0A1EA455F16B58544A21C197AAF4D2E4732965026E5D17F6739C77C69D99B9914278E50E1F0597A6FD5CD72808BE417F3B9E0E7457915DAA85F X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojxgaGEV50waI9LF9SY7GXug== X-Mailru-Sender: 11C2EC085EDE56FA38FD4C59F7EFE407473E51F415CF29A5D990B8260E855F5CFF27A1E6C5FC86D9D51284F0FE6F529ABC7555A253F5B200DF104D74F62EE79D27EC13EC74F6107F4198E0F3ECE9B5443453F38A29522196 X-Mras: OK Subject: Re: [Tarantool-patches] [PATCH 1/2] Fix embedded bytecode loader. 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! Please consider my comments below. On Tue, Jul 25, 2023 at 07:36:24PM +0300, Sergey Bronnikov via Tarantool-patches wrote: > From: sergeyb@tarantool.org > > (cherry-picked from commit 820339960123dc78a7ce03edf53fcf4fdae0e55d) > > Original problem is specific for x32 and is as follows: when a chunk Typo: s/Original/The original/ Typo: s/for x32/to x32/ > with bytecode library is loaded into memory, and the address is higher Typo: s/with bytecode/with a bytecode/ > than 0x80000100, the LexState->pe, that contains an address of the end Typo: s/the LexState/LexState/ > of bytecode chunk in the memory, will wrap around and become smaller Typo: s/of bytecode/of the bytecode/ > than address in LexState->p, that contains an address of the beginning Typo: s/than address/than the address/ > of bytecode chunk in the memory. In bcread_fill() called by > bcread_want(), memcpy() is called with a very large size and causes bus > error on x86 and segmentation fault on ARM android. Typo: s/android/Android/ > > The problem cannot be reproduced on platforms supported by Tarantool > (ARM64, x86_64), so test doesn't reproduce a problem without a patch and > tests patch partially. Typo: s/tests the patch/ Well, I've tried to run that test on x32 machine, and nothing happened. I think we should backport this patch without tests then, since this test seems irrelevant to the problem. What do you think? Also, it is kinda strange, that you are talking about a test in a patch without any tests. You need to either mention that the test is present in the next commit, or move that test here. > > Sergey Bronnikov: > * added the description > --- > src/lj_bcread.c | 10 +++++----- > src/lj_lex.c | 6 ++++++ > src/lj_lex.h | 1 + > 3 files changed, 12 insertions(+), 5 deletions(-) > > diff --git a/src/lj_bcread.c b/src/lj_bcread.c > index f6c7ad25..315ad4d8 100644 > --- a/src/lj_bcread.c > +++ b/src/lj_bcread.c > @@ -79,6 +79,7 @@ static LJ_NOINLINE void bcread_fill(LexState *ls, MSize len, int need) > ls->c = -1; /* Only bad if we get called again. */ > break; > } > + if (sz >= LJ_MAX_BUF - n) lj_err_mem(ls->L); > if (n) { /* Append to buffer. */ > n += (MSize)sz; > p = lj_buf_need(&ls->sb, n < len ? len : n); > @@ -90,20 +91,20 @@ static LJ_NOINLINE void bcread_fill(LexState *ls, MSize len, int need) > ls->p = buf; > ls->pe = buf + sz; > } > - } while (ls->p + len > ls->pe); > + } while ((MSize)(ls->pe - ls->p) < len); > } > > /* Need a certain number of bytes. */ > static LJ_AINLINE void bcread_need(LexState *ls, MSize len) > { > - if (LJ_UNLIKELY(ls->p + len > ls->pe)) > + if (LJ_UNLIKELY((MSize)(ls->pe - ls->p) < len)) > bcread_fill(ls, len, 1); > } > > /* Want to read up to a certain number of bytes, but may need less. */ > static LJ_AINLINE void bcread_want(LexState *ls, MSize len) > { > - if (LJ_UNLIKELY(ls->p + len > ls->pe)) > + if (LJ_UNLIKELY((MSize)(ls->pe - ls->p) < len)) > bcread_fill(ls, len, 0); > } > > @@ -463,8 +464,7 @@ GCproto *lj_bcread(LexState *ls) > setprotoV(L, L->top, pt); > incr_top(L); > } > - if ((int32_t)(2*(uint32_t)(ls->pe - ls->p)) > 0 || > - L->top-1 != bcread_oldtop(L, ls)) > + if ((ls->pe != ls->p && !ls->endmark) || L->top-1 != bcread_oldtop(L, ls)) > bcread_error(ls, LJ_ERR_BCBAD); > /* Pop off last prototype. */ > L->top--; > diff --git a/src/lj_lex.c b/src/lj_lex.c > index 52856912..82e4ba6f 100644 > --- a/src/lj_lex.c > +++ b/src/lj_lex.c > @@ -48,6 +48,11 @@ static LJ_NOINLINE LexChar lex_more(LexState *ls) > size_t sz; > const char *p = ls->rfunc(ls->L, ls->rdata, &sz); > if (p == NULL || sz == 0) return LEX_EOF; > + if (sz >= LJ_MAX_BUF) { > + if (sz != ~(size_t)0) lj_err_mem(ls->L); > + sz = ~(uintptr_t)0 - (uintptr_t)p; > + ls->endmark = 1; > + } > ls->pe = p + sz; > ls->p = p + 1; > return (LexChar)(uint8_t)p[0]; > @@ -406,6 +411,7 @@ int lj_lex_setup(lua_State *L, LexState *ls) > ls->lookahead = TK_eof; /* No look-ahead token. */ > ls->linenumber = 1; > ls->lastline = 1; > + ls->endmark = 0; > lex_next(ls); /* Read-ahead first char. */ > if (ls->c == 0xef && ls->p + 2 <= ls->pe && (uint8_t)ls->p[0] == 0xbb && > (uint8_t)ls->p[1] == 0xbf) { /* Skip UTF-8 BOM (if buffered). */ > diff --git a/src/lj_lex.h b/src/lj_lex.h > index 33fa8657..38d28533 100644 > --- a/src/lj_lex.h > +++ b/src/lj_lex.h > @@ -73,6 +73,7 @@ typedef struct LexState { > BCInsLine *bcstack; /* Stack for bytecode instructions/line numbers. */ > MSize sizebcstack; /* Size of bytecode stack. */ > uint32_t level; /* Syntactical nesting level. */ > + int endmark; /* Trust bytecode end marker, even if not at EOF. */ > } LexState; > > LJ_FUNC int lj_lex_setup(lua_State *L, LexState *ls); > -- > 2.34.1