From e0b95ecf2d6d981478a7609493f4506c6a1236d3 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Wed, 1 Mar 2023 10:34:04 +0100 Subject: [PATCH] Buffer_alloc: initialize the buffer before calling `rb_ivar_set` `rb_ivar_set` may trigger GC, and if it does then we'll try to mark an uninitialized buffer and that will segfault because `b->head == NULL`. --- ChangeLog | 2 ++ ext/msgpack/buffer_class.c | 2 +- spec/cruby/buffer_spec.rb | 11 +++++++++++ spec/packer_spec.rb | 12 ++++++++++++ spec/unpacker_spec.rb | 16 ++++++++++++++++ 5 files changed, 42 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index d8b46183..77befa45 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,5 @@ +* Fix a possible GC crash when GC trigger inside `MessagePack::Buffer.new` (#314). + 2022-09-30 1.6.0: * Fix a potential use-after-free bug in Buffer_free when accessing a packer or unpacker buffer. diff --git a/ext/msgpack/buffer_class.c b/ext/msgpack/buffer_class.c index 6b8581ca..1304321e 100644 --- a/ext/msgpack/buffer_class.c +++ b/ext/msgpack/buffer_class.c @@ -91,8 +91,8 @@ static VALUE Buffer_alloc(VALUE klass) { msgpack_buffer_t* b; VALUE buffer = TypedData_Make_Struct(klass, msgpack_buffer_t, &buffer_data_type, b); - rb_ivar_set(buffer, s_at_owner, Qnil); msgpack_buffer_init(b); + rb_ivar_set(buffer, s_at_owner, Qnil); return buffer; } diff --git a/spec/cruby/buffer_spec.rb b/spec/cruby/buffer_spec.rb index 790090d2..fe1d51b6 100644 --- a/spec/cruby/buffer_spec.rb +++ b/spec/cruby/buffer_spec.rb @@ -589,4 +589,15 @@ buffer.read_all expect(ObjectSpace.memsize_of(buffer)).to be == empty_size end + + it "doesn't crash when marking an uninitialized buffer" do + stress = GC.stress + begin + GC.stress = true + + MessagePack::Buffer.new + ensure + GC.stress = stress + end + end end diff --git a/spec/packer_spec.rb b/spec/packer_spec.rb index cae60be1..2b1df27e 100644 --- a/spec/packer_spec.rb +++ b/spec/packer_spec.rb @@ -572,4 +572,16 @@ def to_msgpack_ext [0xc9, 65538, -1].pack('CNC') + "a"*65538 end end + + it "doesn't crash when marking an uninitialized buffer" do + stress = GC.stress + begin + GC.stress = true + + MessagePack::Packer.new.buffer + Object.new + ensure + GC.stress = stress + end + end end diff --git a/spec/unpacker_spec.rb b/spec/unpacker_spec.rb index 7d5d28b8..cb51aa65 100644 --- a/spec/unpacker_spec.rb +++ b/spec/unpacker_spec.rb @@ -866,4 +866,20 @@ def flatten(struct, results = []) end end end + + it "doesn't crash when marking an uninitialized buffer" do + if RUBY_PLATFORM == "java" + pending("THe java extension is missing Unpacker#buffer https://github.com/msgpack/msgpack-ruby/issues/315") + end + + stress = GC.stress + begin + GC.stress = true + + MessagePack::Unpacker.new.buffer + Object.new + ensure + GC.stress = stress + end + end end