Delete copy constructor for stack_frame_guard and stack_frame

The copy constructors are causing problems when the `fiber.stack` vector
gets reallocated when its capacity is full, since when vectors are
reallocated, the elements are moved (or copied if there's no usable move
constructor) to the reallocated memory and then the original elements
are destroyed.

This premature calling of destructors leads to double-free and
use-after-free errors.

I fixed it by deleting the copy constructors and explicitly defining
move constructors.
This commit is contained in:
刘皓 2025-04-16 21:39:04 -04:00
parent 8e9e7700f0
commit 5a5fcd26c5
No known key found for this signature in database
GPG key ID: 7901753DB465B711
7 changed files with 69 additions and 44 deletions

View file

@ -37,10 +37,21 @@
using namespace mkxp_sandbox;
binding_base::stack_frame::stack_frame(struct binding_base &bind, void (*destructor)(void *ptr), boost::typeindex::type_index type, wasm_ptr_t ptr) : bind(bind), destructor(destructor), type(type), ptr(ptr) {}
binding_base::stack_frame::stack_frame(struct binding_base &bind, void (*destructor)(void *ptr), wasm_ptr_t ptr) : bind(&bind), destructor(destructor), ptr(ptr) {}
binding_base::stack_frame::stack_frame(struct binding_base::stack_frame &&frame) noexcept : bind(std::exchange(frame.bind, nullptr)), destructor(std::exchange(frame.destructor, nullptr)), ptr(std::exchange(frame.ptr, 0)) {}
struct binding_base::stack_frame &binding_base::stack_frame::operator=(struct binding_base::stack_frame &&frame) noexcept {
bind = std::exchange(frame.bind, nullptr);
destructor = std::exchange(frame.destructor, nullptr);
ptr = std::exchange(frame.ptr, 0);
return *this;
}
binding_base::stack_frame::~stack_frame() {
destructor(*bind + ptr);
if (destructor != nullptr) {
destructor(**bind + ptr);
}
}
binding_base::binding_base(std::shared_ptr<struct w2c_ruby> m) : next_func_ptr(-1), _instance(m) {}

View file

@ -27,9 +27,9 @@
#include <cstring>
#include <memory>
#include <unordered_map>
#include <utility>
#include <vector>
#include <boost/container_hash/hash.hpp>
#include <boost/type_index.hpp>
#include <boost/asio/coroutine.hpp>
#include <mkxp-retro-ruby.h>
#include "types.h"
@ -59,16 +59,18 @@
namespace mkxp_sandbox {
struct binding_base {
private:
private:
typedef std::tuple<wasm_ptr_t, wasm_ptr_t, wasm_ptr_t> key_t;
struct stack_frame {
struct binding_base &bind;
struct binding_base *bind;
void (*destructor)(void *ptr);
boost::typeindex::type_index type;
wasm_ptr_t ptr;
stack_frame(struct binding_base &bind, void (*destructor)(void *ptr), boost::typeindex::type_index type, wasm_ptr_t ptr);
stack_frame(struct binding_base &bind, void (*destructor)(void *ptr), wasm_ptr_t ptr);
stack_frame(const struct stack_frame &frame) = delete;
stack_frame(struct stack_frame &&frame) noexcept;
struct stack_frame &operator=(const struct stack_frame &frame) = delete;
struct stack_frame &operator=(struct stack_frame &&frame) noexcept;
~stack_frame();
};
@ -82,8 +84,7 @@ namespace mkxp_sandbox {
std::shared_ptr<struct w2c_ruby> _instance;
std::unordered_map<key_t, struct fiber, boost::hash<key_t>> fibers;
public:
public:
binding_base(std::shared_ptr<struct w2c_ruby> m);
~binding_base();
struct w2c_ruby &instance() const noexcept;
@ -100,10 +101,9 @@ namespace mkxp_sandbox {
template <typename T> struct stack_frame_guard {
friend struct binding_base;
private:
struct binding_base &bind;
struct fiber &fiber;
private:
struct binding_base *bind;
struct fiber *fiber;
wasm_ptr_t ptr;
static void stack_frame_destructor(void *ptr) {
@ -134,7 +134,6 @@ namespace mkxp_sandbox {
if (fiber.stack_ptr == fiber.stack.size()) {
std::abort();
}
assert(fiber.stack[fiber.stack_ptr].type == boost::typeindex::type_id<T>());
return fiber.stack[fiber.stack_ptr++].ptr;
}
@ -145,10 +144,10 @@ namespace mkxp_sandbox {
}
++fiber.stack_ptr;
wasm_ptr_t sp = w2c_ruby_rb_wasm_get_stack_pointer(&bind.instance()) - SIZEOF_WASMSTACKALIGN(T);
fiber.stack.reserve(fiber.stack_ptr);
fiber.stack.emplace_back(
bind,
stack_frame_destructor,
boost::typeindex::type_id<T>(),
sp
);
assert(sp % sizeof(VALUE) == 0);
@ -158,33 +157,47 @@ namespace mkxp_sandbox {
return sp;
}
stack_frame_guard(struct binding_base &b) : bind(b), fiber(init_fiber(b)), ptr(init_inner(b, fiber)) {}
stack_frame_guard(struct binding_base &b) : bind(&b), fiber(&init_fiber(b)), ptr(init_inner(b, *fiber)) {}
public:
public:
stack_frame_guard(const stack_frame_guard &frame) = delete;
stack_frame_guard(stack_frame_guard &&frame) noexcept : bind(std::exchange(frame.bind, nullptr)), fiber(std::exchange(frame.fiber, nullptr)), ptr(std::exchange(frame.ptr, 0)) {}
struct stack_frame_guard &operator=(const stack_frame_guard &frame) = delete;
struct stack_frame_guard &operator=(stack_frame_guard &&frame) noexcept {
bind = std::exchange(frame.bind, nullptr);
fiber = std::exchange(frame.fiber, nullptr);
ptr = std::exchange(frame.ptr, 0);
return *this;
}
~stack_frame_guard() {
if (get()->is_complete()) {
while (fiber.stack.size() > fiber.stack_ptr) {
fiber.stack.pop_back();
}
// Check for stack corruptions
assert(fiber.stack.size() == fiber.stack_ptr);
assert(fiber.stack.back().type == boost::typeindex::type_id<T>());
w2c_ruby_rb_wasm_set_stack_pointer(&bind.instance(), fiber.stack.back().ptr + SIZEOF_WASMSTACKALIGN(T));
fiber.stack.pop_back();
if (fiber == nullptr) {
return;
}
--fiber.stack_ptr;
if (get()->is_complete()) {
while (fiber->stack.size() > fiber->stack_ptr) {
fiber->stack.pop_back();
}
if (fiber.stack.empty()) {
bind.fibers.erase(fiber.key);
assert(fiber->stack.size() == fiber->stack_ptr);
w2c_ruby_rb_wasm_set_stack_pointer(&bind->instance(), fiber->stack.back().ptr + SIZEOF_WASMSTACKALIGN(T));
fiber->stack.pop_back();
}
--fiber->stack_ptr;
if (fiber->stack.empty()) {
bind->fibers.erase(fiber->key);
}
}
inline T *get() const noexcept {
return (T *)(*bind + ptr);
return (T *)(**bind + ptr);
}
inline T &operator()() const noexcept {

View file

@ -233,20 +233,27 @@ namespace mkxp_sandbox {
VALUE value;
VALUE file;
wasm_ptr_t ptr;
std::string path_str;
std::string *path_str;
VALUE operator()(VALUE path) {
BOOST_ASIO_CORO_REENTER (this) {
path_str = NULL;
SANDBOX_AWAIT_AND_SET(ptr, rb_string_value_cstr, &path);
path_str = std::string("/mkxp-retro-game/");
path_str.append((const char *)(**sb() + ptr));
SANDBOX_AWAIT_AND_SET(file, rb_file_open, path_str.c_str(), "rb");
path_str = new std::string("/mkxp-retro-game/");
path_str->append((const char *)(**sb() + ptr));
SANDBOX_AWAIT_AND_SET(file, rb_file_open, path_str->c_str(), "rb");
SANDBOX_AWAIT_AND_SET(value, rb_marshal_load, file);
SANDBOX_AWAIT(rb_io_close, file);
}
return value;
}
~coro() {
if (path_str != NULL) {
delete path_str;
}
}
)
return sb()->bind<struct coro>()()(path);

View file

@ -27,7 +27,7 @@
#include <mkxp-sandbox-bindgen.h>
#include "types.h"
#define SANDBOX_COROUTINE(name, definition) struct name : boost::asio::coroutine { BOOST_TYPE_INDEX_REGISTER_CLASS inline name(struct mkxp_sandbox::binding_base &bind) {} definition };
#define SANDBOX_COROUTINE(name, definition) struct name : boost::asio::coroutine { inline name(struct mkxp_sandbox::binding_base &bind) {} definition };
#define SANDBOX_AWAIT(coroutine, ...) \
do { \

View file

@ -617,7 +617,6 @@ File.readlines('tags', chomp: true).each do |line|
coroutine_declaration = <<~HEREDOC
struct #{func_name} : boost::asio::coroutine {
friend struct bindings::stack_frame_guard<struct #{func_name}>;
BOOST_TYPE_INDEX_REGISTER_CLASS
#{coroutine_ret} operator()(#{declaration_args.join(', ')});
#{coroutine_destructor.empty? ? '' : "~#{func_name}();\n "}private:
struct binding_base &bind;

View file

@ -47,7 +47,6 @@ if is_libretro
cmake.subproject('boost_throw_exception', options: boost_options).dependency('boost_throw_exception'),
cmake.subproject('boost_core', options: boost_options).dependency('boost_core'),
cmake.subproject('boost_container_hash', options: boost_options).dependency('boost_container_hash'),
cmake.subproject('boost_type_index', options: boost_options).dependency('boost_type_index'),
cmake.subproject('boost_type_traits', options: boost_options).dependency('boost_type_traits'),
cmake.subproject('boost_optional', options: boost_options).dependency('boost_optional'),
]

View file

@ -1,4 +0,0 @@
[wrap-git]
url = https://github.com/boostorg/type_index
revision = boost-1.87.0
depth = 1