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

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

[Sergey Kaplun via Tarantool-patches]

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)
-- 
2.56.0

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

[Evgeniy Temirgaleev via Tarantool-patches]

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