Re: [Tarantool-patches] [PATCH luajit] Clarify that lj_buf_shrink() does not keep any buffer data.

[Evgeniy Temirgaleev via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi, Sergey! Thanks for the patch! LGTM

On 11/11/26 4:38 PM, Sergey Kaplun wrote:
> From: Mike Pall <mike>
> 
> https://www.freelists.org/post/luajit/lj-buf-shrink-may-truncate-the-data-and-sbw-point-over-the-end-of-the-buffer,1
> Thanks to Junlong li.
> 
> (cherry picked from commit 444c8ff19a39248307a7037354d6d259cc0e5efc)
> 
> When `lj_cf_string_format()` formats any object to the string via `%s`
> it casts this object to a string implicitly on the first pass of parsing
> the regexp. If the `__tostring` metamethod of the object invokes the
> atomic step of the GC, GC shrinks the `tmpbuf` is used for string
> formatting. In case when the result string occupies more than half of
> the buffer, the writes in the second pass miss the buffer and may lead
> to the heap-buffer-overflow, an invalid result string, or the core dump.
> 
> This patch resets the current pointer position in the buffer in the
> `lj_buf_shrink()` to avoid incorrect buffer writes.
> 
> Sergey Kaplun:
> * added the description and the test for the problem
> 
> Resolves tarantool/tarantool#4171
> Part of tarantool/tarantool#13442
> ---
> 
> Branch: https://github.com/tarantool/luajit/tree/skaplun/gh-4171-lj-cf-string-format-atomic
> Related issues:
> * https://github.com/tarantool/tarantool/issues/4171
> * https://github.com/tarantool/tarantool/issues/13442
> 
>   src/lj_buf.c                                  |  3 +-
>   ...h-4171-lj-cf-string-format-atomic.test.lua | 30 +++++++++++++++++++
>   2 files changed, 31 insertions(+), 2 deletions(-)
>   create mode 100644 test/tarantool-tests/gh-4171-lj-cf-string-format-atomic.test.lua
> 
> diff --git a/src/lj_buf.c b/src/lj_buf.c
> index 923f4276..bcfc4c93 100644
> --- a/src/lj_buf.c
> +++ b/src/lj_buf.c
> @@ -52,10 +52,9 @@ void LJ_FASTCALL lj_buf_shrink(lua_State *L, SBuf *sb)
>     char *b = sbufB(sb);
>     MSize osz = (MSize)(sbufE(sb) - b);
>     if (osz > 2*LJ_MIN_SBUF) {
> -    MSize n = (MSize)(sbufP(sb) - b);
>       b = lj_mem_realloc(L, b, osz, (osz >> 1));
>       setmref(sb->b, b);
> -    setmref(sb->p, b + n);
> +    setmref(sb->p, b);  /* Not supposed to keep data across shrinks. */
>       setmref(sb->e, b + (osz >> 1));
>     }
>   }
> diff --git a/test/tarantool-tests/gh-4171-lj-cf-string-format-atomic.test.lua b/test/tarantool-tests/gh-4171-lj-cf-string-format-atomic.test.lua
> new file mode 100644
> index 00000000..c601ce08
> --- /dev/null
> +++ b/test/tarantool-tests/gh-4171-lj-cf-string-format-atomic.test.lua
> @@ -0,0 +1,30 @@
> +local tap = require('tap')
> +
> +-- The test file demonstrates `string.format()`
> +-- heap-buffer-overflow under ASAN if the atomic phase of the GC
> +-- is entered during the first pass of formatting with implicit
> +-- conversion of the argument to a string via %s.
> +-- See also: https://github.com/tarantool/tarantool/issues/4171.
> +local test = tap.test('gh-4171-lj-cf-string-format-atomic')
> +
> +test:plan(1)
> +
> +local obj = setmetatable({}, {
> +  __tostring = function()
> +    -- Shrinks `tmpbuf`: 256 -> 128 (but length of string is 200).
> +    collectgarbage()
> +    return 'x'
> +  end
> +})
> +
> +local prefix = string.rep('a', 200)
> +
> +-- %s triggers a full GC cycle, including `tmpbuf` shrinking in
> +-- the atomic phase. '_suffix' was written outside the buffer
> +-- before the patch.
> +local res = string.format(prefix .. '%s_suffix', obj)
> +
> +-- Under ASAN this leads to heap-buffer-overflow.
> +test:is(res, prefix .. 'x_suffix', 'correct string format result')
> +
> +test:done(true)

-- 
--
Best regards,
Evgeniy Temirgaleev