diff --git a/changelogs/unreleased/fix-netbox-memory-corruption.md b/changelogs/unreleased/fix-netbox-memory-corruption.md new file mode 100644 index 0000000000000000000000000000000000000000..77cc8b56295e90632992a0b0b9faf76386424fa5 --- /dev/null +++ b/changelogs/unreleased/fix-netbox-memory-corruption.md @@ -0,0 +1,4 @@ +## bugfix/core + +* Fixed memory corruption in netbox. Because of the wrong order of the ffi.gc + and ffi.cast calls memory of struct error, which was still used, was freed \ No newline at end of file diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua index 82709e16a700223cc70be12bc65076e4b160e169..0e4f263dc01b7fc57dffafe5a547663d431fe8aa 100644 --- a/src/box/lua/net_box.lua +++ b/src/box/lua/net_box.lua @@ -152,12 +152,12 @@ local function decode_error(raw_data) local err = ffi.C.error_unpack_unsafe(ptr) if err ~= nil then err._refs = err._refs + 1 - err = ffi.gc(err, ffi.C.error_unref) -- From FFI it is returned as 'struct error *', which is -- not considered equal to 'const struct error &', and is -- is not accepted by functions like box.error(). Need to -- cast explicitly. err = ffi.cast('const struct error &', err) + err = ffi.gc(err, ffi.C.error_unref) else -- Error unpacker installs fail reason into diag. box.error() diff --git a/test/box/net.box_memory_corruption.result b/test/box/net.box_memory_corruption.result new file mode 100644 index 0000000000000000000000000000000000000000..a4bb233c2e68852e5b659871d0fbe60d72a45e35 --- /dev/null +++ b/test/box/net.box_memory_corruption.result @@ -0,0 +1,56 @@ +-- test-run result file version 2 +-- There was a bug in the netbox module related to access +-- to previously released memory. To understand the essence +-- of error, you need to understand how GC works in Lua: +-- - GC checks the reachability of objects in Lua in one cycle +-- and cleans out those that were unreachable +-- - Lua GC object is an entity whose memory is managed by the GC, +-- for example: table, function, userdata, cdata. +-- In our case it's cdata object, with struct error payload +-- - ffi.gc allows us to clean up Lua GC object payload at the time +-- of deleting the GC object. +-- - Finalizer in ffi.gc is hung on the Lua GC object + +-- So after ffi.cast in our case first err object becomes unreachable. +-- It will be cleaned after some time and if finalizer hangs on it, +-- payload will also be cleaned. So payload in new err object +-- (struct error in our case) becomes invalid. + +env = require('test_run') + | --- + | ... +netbox = require('net.box') + | --- + | ... +test_run = env.new() + | --- + | ... +box.schema.user.grant('guest', 'super') + | --- + | ... + +c = netbox.connect(box.cfg.listen) + | --- + | ... +collectgarbage() + | --- + | - 0 + | ... +ok, res = pcall(c.call, c, 'box.error', {123}) + | --- + | ... +box.error.clear() + | --- + | ... +collectgarbage() + | --- + | - 0 + | ... +res.type + | --- + | - ClientError + | ... +box.schema.user.revoke('guest', 'super') + | --- + | ... + diff --git a/test/box/net.box_memory_corruption.test.lua b/test/box/net.box_memory_corruption.test.lua new file mode 100644 index 0000000000000000000000000000000000000000..2c9993f21c8072f4ac312532017ad075397483c6 --- /dev/null +++ b/test/box/net.box_memory_corruption.test.lua @@ -0,0 +1,30 @@ +-- There was a bug in the netbox module related to access +-- to previously released memory. To understand the essence +-- of error, you need to understand how GC works in Lua: +-- - GC checks the reachability of objects in Lua in one cycle +-- and cleans out those that were unreachable +-- - Lua GC object is an entity whose memory is managed by the GC, +-- for example: table, function, userdata, cdata. +-- In our case it's cdata object, with struct error payload +-- - ffi.gc allows us to clean up Lua GC object payload at the time +-- of deleting the GC object. +-- - Finalizer in ffi.gc is hung on the Lua GC object + +-- So after ffi.cast in our case first err object becomes unreachable. +-- It will be cleaned after some time and if finalizer hangs on it, +-- payload will also be cleaned. So payload in new err object +-- (struct error in our case) becomes invalid. + +env = require('test_run') +netbox = require('net.box') +test_run = env.new() +box.schema.user.grant('guest', 'super') + +c = netbox.connect(box.cfg.listen) +collectgarbage() +ok, res = pcall(c.call, c, 'box.error', {123}) +box.error.clear() +collectgarbage() +res.type +box.schema.user.revoke('guest', 'super') +