Skip to content

Commit

Permalink
Fix limit check in narrow_conv_backprop().
Browse files Browse the repository at this point in the history
Thanks to Sergey Kaplun.

(cherry picked from commit e45fd4c)

The function `narrow_conv_backprop()` does not include a limit check for
the stack pointer (`nc->sp`), even though its value may change after
several recursive calls. As a result, it leads to stack-buffer-overflow
during the instruction narrowing. This patch adds the missing check.

Sergey Kaplun:
* added the description and the test for the problem

Part of tarantool/tarantool#10199
  • Loading branch information
Mike Pall authored and Buristan committed Sep 23, 2024
1 parent 4b0f172 commit 9b784c2
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 1 deletion.
3 changes: 2 additions & 1 deletion src/lj_opt_narrow.c
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,8 @@ static int narrow_conv_backprop(NarrowConv *nc, IRRef ref, int depth)
NarrowIns *savesp = nc->sp;
int count = narrow_conv_backprop(nc, ir->op1, depth);
count += narrow_conv_backprop(nc, ir->op2, depth);
if (count <= 1) { /* Limit total number of conversions. */
/* Limit total number of conversions. */
if (count <= 1 && nc->sp < nc->maxsp) {
*nc->sp++ = NARROWINS(IRT(ir->o, nc->t), ref);
return count;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
local tap = require('tap')

-- Test file to demonstrate stack-buffer-overflow during the
-- narrowing optimization.
-- See also: https://github.com/LuaJIT/LuaJIT/issues/1262.

local test = tap.test('lj-1262-fix-limit-narrow-conv-backprop'):skipcond({
['Test requires JIT enabled'] = not jit.status(),
})

test:plan(1)

-- XXX: Test fails only under ASAN.
-- XXX: The original reproducer was found by fuzzer:
-- https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=70779.
-- It creates a long side trace with a huge amount of ADD IRs,
-- which are recursively used in the `narrow_conv_backprop()` many
-- enough times to catch the stack-buffer-overflow. I can't more
-- simplify the reproducer any (or write it from scratch), so I
-- leave it in that state.

local DEFAULT_NUMBER = 1
local tonumber = tonumber

local always_number = function(val)
return tonumber(val) or DEFAULT_NUMBER
end

local add = function(v1, v2)
return always_number(v1) + always_number(v2)
end

jit.opt.start('hotloop=1', 'hotexit=1')

local counter_0 = 0
local counter_1 = 0
local counter_2 = 0
local tmp = add(nil, 'Name')
local Name0 = add(tmp, 'Name')
-- Start a long side trace here.
for _ = 0, 0, 0 do
if counter_0 > 5 then break end
counter_0 = counter_0 + 1

for _ = always_number(false), 1, always_number(Name0) do
if counter_1 > 5 then break end
counter_1 = counter_1 + 1

repeat
if counter_2 > 5 then break end
counter_2 = counter_2 + 1

Name0 = Name0 + Name0 + Name0
Name0 = add(Name0, nil) + Name0
until nil
end
end

test:ok(true, 'no stack-buffer-overflow during narrowing')

test:done(true)

0 comments on commit 9b784c2

Please sign in to comment.