netbox: fix memory corruption in net.box module

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.

changelogs/unreleased/fix-netbox-memory-corruption.md

+## 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

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

test/box/net.box_memory_corruption.result

+-- 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')
+ | ---
+ | ...
+

test/box/net.box_memory_corruption.test.lua

+-- 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')
+