Tarantool development patches archive
 help / color / mirror / Atom feed
From: Maxim Kokryashkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Sergey Bronnikov <estetus@gmail.com>
Cc: max.kokryashkin@gmail.com, tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH luajit 2/2][v2] Followup fix for embedded bytecode loader.
Date: Fri, 1 Sep 2023 13:05:11 +0300	[thread overview]
Message-ID: <yeiqzojpelto6wdubzk76gpkretkzruhnk2h45jbe34ljfevyq@6h4bpqrotzi2> (raw)
In-Reply-To: <bdc81063aad97cea7d252e68a263759fd34bbc78.1693480177.git.sergeyb@tarantool.org>

On Thu, Aug 31, 2023 at 02:32:14PM +0300, Sergey Bronnikov via Tarantool-patches wrote:
> From: Sergey Bronnikov <sergeyb@tarantool.org>
> 
> (cherry-picked from commit e49863eda13d095b1a78fd4ca0fd3a6a9a17d782)
> 
> The patch follows up a previous patch and limits the total size of a
> chunk load by `lua_load` with size `LJ_MAX_BUF - 1`.
> 
> Sergey Bronnikov:
> * added the description and the test
> ---
>  src/lj_lex.c                                  |   1 +
>  test/tarantool-c-tests/lj-549-lua_load.test.c | 134 ++++++++++++++++++
>  2 files changed, 135 insertions(+)
>  create mode 100644 test/tarantool-c-tests/lj-549-lua_load.test.c
> 
> diff --git a/src/lj_lex.c b/src/lj_lex.c
> index 6291705f..13495c41 100644
> --- a/src/lj_lex.c
> +++ b/src/lj_lex.c
> @@ -51,6 +51,7 @@ static LJ_NOINLINE LexChar lex_more(LexState *ls)
>    if (sz >= LJ_MAX_BUF) {
>      if (sz != ~(size_t)0) lj_err_mem(ls->L);
>      sz = ~(uintptr_t)0 - (uintptr_t)p;
> +    if (sz >= LJ_MAX_BUF) sz = LJ_MAX_BUF-1;
>      ls->endmark = 1;
>    }
>    ls->pe = p + sz;
> diff --git a/test/tarantool-c-tests/lj-549-lua_load.test.c b/test/tarantool-c-tests/lj-549-lua_load.test.c
> new file mode 100644
> index 00000000..9baa7a1a
> --- /dev/null
> +++ b/test/tarantool-c-tests/lj-549-lua_load.test.c
> @@ -0,0 +1,134 @@
> +#include <assert.h>
> +#include <stdint.h>
> +#include <stddef.h>
> +#include <string.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +
> +#include <lua.h>
> +#include <lualib.h>
> +#include <lauxlib.h>
> +
> +#include "test.h"
> +#include "utils.h"
> +
> +/* Need for skipcond. */
> +#include "lj_arch.h"
> +
> +/* Defined in lj_def.h. */
> +#define LJ_MAX_MEM32    0x7fffff00      /* Max. 32 bit memory allocation. */
> +#define LJ_MAX_BUF      LJ_MAX_MEM32    /* Max. buffer length. */
> +
> +/* Defined in lua.h. */
> +/* mark for precompiled code (`<esc>Lua') */
> +#define	LUA_SIGNATURE	"\033Lua"
> +
> +#define UNUSED(x) ((void)(x))
> +
> +/**
> + * Function generates a huge chunk of "bytecode" with a size bigger than
> + * LJ_MAX_BUF. Generated chunk must enable endmark in a Lex state.
> + */
> +static const char *
> +bc_reader_with_endmark(lua_State *L, void *data, size_t *size)
> +{
> +	UNUSED(data);
> +	int bc_chunk_size = (size_t)0;
> +	static char *bc_chunk = NULL;
> +	free(bc_chunk);
What's the point of free here? Why the buffer is static?
> +
> +	bc_chunk = malloc(bc_chunk_size);
Malloc of zero size doesn't seem to be the thing you wanted to do.
> +	assert(bc_chunk != NULL);
> +
> +	/**
> +	 * `lua_load` automatically detects whether the chunk is text or binary,
> +	 * and loads it accordingly. We need a trace for bytecode input,
> +	 * so it is necessary to deceive a check in lj_lex_setup, that
> +	 * makes a sanity check and detects whether input is bytecode or text
> +	 * by the first char. Put LUA_SIGNATURE[0] at the beginning of the
> +	 * allocated region.
> +	 */
> +	bc_chunk[0] = LUA_SIGNATURE[0];
> +
> +	*size = bc_chunk_size;
> +
> +	return bc_chunk;
> +}
> +
> +static int bc_loader_with_endmark(void *test_state)
> +{
> +	lua_State *L = test_state;
> +	void *ud = NULL;
> +	int res = lua_load(L, bc_reader_with_endmark, ud, "endmark");
> +
> +	/*
> +	 * Make sure we passed the condition with lj_err_mem in the function
> +	 * `lex_more`.
> +	 */
> +	assert_true(res != LUA_ERRMEM);
> +
> +	return TEST_EXIT_SUCCESS;
> +}
> +
> +enum bc_emission_state {
> +	EMIT_BC,
> +	EMIT_EOF,
> +};
> +
> +typedef struct {
> +	enum bc_emission_state state;
> +} dt;
> +
> +/**
> + * Function returns a bytecode chunk on the first call and NULL and size equal
> + * to zero on the second call. Triggers the END_OF_STREAM flag in the function
> + * `lex_more`.
> + */
> +static const char *
> +bc_reader_with_eof(lua_State *L, void *data, size_t *size)
> +{
> +	UNUSED(data);
> +	UNUSED(L);
> +	dt *test_data = (dt *)data;
> +	if (test_data->state == EMIT_EOF) {
This section is unreachable, isn't it?
> +		*size = 0;
> +		return NULL;
> +	}
> +
> +	static char *bc_chunk = NULL;
> +	free(bc_chunk);
Ditto.
> +
> +	size_t sz = 10;
Is there any reason for it to be exactly 10? Drop a comment.
> +	bc_chunk = malloc(sz);
> +	bc_chunk[0] = LUA_SIGNATURE[0];
> +	*size = sz;
> +
> +	return bc_chunk;
> +}
> +
> +static int bc_loader_with_eof(void *test_state)
> +{
> +	lua_State *L = test_state;
> +	dt test_data = {0};
> +	test_data.state = EMIT_BC;
> +	int res = lua_load(L, bc_reader_with_eof, &test_data, "eof");
> +	assert_true(res = LUA_ERRSYNTAX);
> +	if (res == LUA_OK) {
> +		lua_pcall(L, 0, 0, 0);
> +	}
> +
> +	return TEST_EXIT_SUCCESS;
> +}
> +
> +int main(void)
> +{
> +	lua_State *L = utils_lua_init();
> +	const struct test_unit tgroup[] = {
> +		test_unit_def(bc_loader_with_endmark),
> +		test_unit_def(bc_loader_with_eof)
> +	};
> +
> +	const int test_result = test_run_group(tgroup, L);
> +	utils_lua_close(L);
> +	return test_result;
> +}
> -- 
> 2.34.1
> 

  reply	other threads:[~2023-09-01 10:05 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-31 11:29 [Tarantool-patches] [PATCH luajit 0/2][v2] Fix " Sergey Bronnikov via Tarantool-patches
2023-08-31 11:30 ` [Tarantool-patches] [PATCH luajit 1/2][v2] " Sergey Bronnikov via Tarantool-patches
2023-08-31 11:49   ` Sergey Bronnikov via Tarantool-patches
2023-09-01  9:42   ` Maxim Kokryashkin via Tarantool-patches
2023-09-04  9:31     ` Sergey Bronnikov via Tarantool-patches
2023-09-05  6:34       ` Maxim Kokryashkin via Tarantool-patches
2023-09-05 14:10   ` Sergey Kaplun via Tarantool-patches
2023-09-07 15:21     ` Sergey Bronnikov via Tarantool-patches
2023-09-11  8:45       ` Sergey Kaplun via Tarantool-patches
2023-09-12 10:20         ` Sergey Bronnikov via Tarantool-patches
2023-10-31 11:30           ` Sergey Kaplun via Tarantool-patches
2023-09-05 14:12   ` Sergey Kaplun via Tarantool-patches
2023-09-07  7:06     ` Sergey Bronnikov via Tarantool-patches
2023-08-31 11:32 ` [Tarantool-patches] [PATCH luajit 2/2][v2] Followup fix for " Sergey Bronnikov via Tarantool-patches
2023-09-01 10:05   ` Maxim Kokryashkin via Tarantool-patches [this message]
2023-09-04 16:34     ` Sergey Bronnikov via Tarantool-patches
2023-09-05  6:45       ` Maxim Kokryashkin via Tarantool-patches
2023-09-05 12:55   ` Sergey Kaplun via Tarantool-patches
2023-09-07  7:04     ` Sergey Bronnikov via Tarantool-patches
2023-09-11  9:26       ` Sergey Kaplun via Tarantool-patches
2023-09-12 10:30         ` Sergey Bronnikov via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=yeiqzojpelto6wdubzk76gpkretkzruhnk2h45jbe34ljfevyq@6h4bpqrotzi2 \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=estetus@gmail.com \
    --cc=m.kokryashkin@tarantool.org \
    --cc=max.kokryashkin@gmail.com \
    --subject='Re: [Tarantool-patches] [PATCH luajit 2/2][v2] Followup fix for embedded bytecode loader.' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox