Skip to content

Commit 3f8071d

Browse files
committed
[Bug #22240] Protect against rb_jump_tag(TAG_RAISE) after errinfo has been cleared
If native code calls rb_protect, then calls rb_funcall before calling rb_jump_tag, and that ruby code has a rescue clause, it may set errinfo to nil. Previously this would cause a SEGV as we weren't checking the type and assuming errinfo was an object. The exception that was being raised is gone at that point, so there is nothing to recover from: report the broken state with rb_bug instead of unwinding with an errinfo that isn't an exception object.
1 parent c41b67d commit 3f8071d

3 files changed

Lines changed: 100 additions & 0 deletions

File tree

‎ext/-test-/exception/nil_errinfo.c‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
#include <ruby.h>
2+
3+
static VALUE
4+
raise_boom(VALUE _arg)
5+
{
6+
rb_raise(rb_eRuntimeError, "boom");
7+
UNREACHABLE_RETURN(Qnil);
8+
}
9+
10+
/*
11+
* If a native extension catches an exception with rb_protect()
12+
* and then uses rb_funcall to run Ruby code that rescues an exception
13+
* the errinfo will be set to Qnil before the extension calls rb_jump_tag.
14+
* The exception is lost, so the VM reports it with rb_bug().
15+
*
16+
* Extensions should save the errinfo before the call and restore it afterward.
17+
*/
18+
static VALUE
19+
raise_after_rescue_cleanup(VALUE self)
20+
{
21+
int state = 0;
22+
23+
rb_protect(raise_boom, Qnil, &state);
24+
25+
if (state) {
26+
/* Any begin/rescue that catches an exception clears ec->errinfo. */
27+
rb_funcall(self, rb_intern("cleanup_with_rescue"), 0);
28+
/* ec->errinfo is now Qnil, but state is still TAG_RAISE. */
29+
rb_jump_tag(state);
30+
}
31+
return Qnil;
32+
}
33+
34+
void
35+
Init_nil_errinfo(VALUE klass)
36+
{
37+
rb_define_singleton_method(klass, "raise_after_rescue_cleanup",
38+
raise_after_rescue_cleanup, 0);
39+
}
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
# frozen_string_literal: false
2+
require 'test/unit'
3+
require 'tmpdir'
4+
require '-test-/exception'
5+
6+
module Bug
7+
class Test_ExceptionNilErrinfo < Test::Unit::TestCase
8+
# A native extension that runs Ruby code containing a rescue clause
9+
# between rb_protect() and rb_jump_tag() clears ec->errinfo,
10+
# losing the exception that was being raised.
11+
# There is nothing left to raise, so report it instead of unwinding.
12+
def test_rescue_cleanup_is_a_bug
13+
no_core = "Process.setrlimit(Process::RLIMIT_CORE, 0); " if defined?(Process.setrlimit) && defined?(Process::RLIMIT_CORE)
14+
src = <<~'RUBY'
15+
def (Bug::Exception).cleanup_with_rescue
16+
begin
17+
raise "cleanup error"
18+
rescue
19+
# entering this rescue sets ec->errinfo to Qnil
20+
end
21+
end
22+
23+
Bug::Exception.raise_after_rescue_cleanup
24+
RUBY
25+
expected_stderr = [
26+
:*,
27+
/\[BUG\]\sexception object was lost during stack unwinding/,
28+
:*,
29+
]
30+
Dir.mktmpdir do |tmpdir|
31+
args = [{"RUBY_ON_BUG" => nil, "RUBY_CRASH_REPORT" => nil}, "-r-test-/exception", "-C", tmpdir]
32+
# Writing the report is slow, see TestRubyOptions#assert_segv.
33+
assert_in_out_err(args, "#{no_core}#{src}", [], expected_stderr, encoding: "ASCII-8BIT", timeout: 60)
34+
end
35+
end
36+
37+
# Without a rescue clause errinfo is not cleared,
38+
# so the exception raised by the cleanup code propagates as usual.
39+
def test_rescue_cleanup_raises_latest_error
40+
assert_in_out_err(["-r-test-/exception"], <<~'RUBY', [], /cleanup error \(RuntimeError\)/)
41+
def (Bug::Exception).cleanup_with_rescue
42+
raise "cleanup error"
43+
end
44+
45+
Bug::Exception.raise_after_rescue_cleanup
46+
RUBY
47+
end
48+
end
49+
end

‎vm.c‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3025,6 +3025,18 @@ vm_exec_handle_exception(rb_execution_context_t *ec, enum ruby_tag_type state, V
30253025
{
30263026
struct vm_throw_data *err = (struct vm_throw_data *)errinfo;
30273027

3028+
/* If a native extension runs Ruby code between rb_protect() and
3029+
* rb_jump_tag() and that code rescues an exception, ec->errinfo is
3030+
* cleared while the tag state is still TAG_RAISE. The exception that
3031+
* was being raised is lost and unwinding would dereference a non object,
3032+
* so there is nothing to recover from. Extensions must save errinfo
3033+
* before such a call and restore it before rb_jump_tag(). */
3034+
if (state == TAG_RAISE && RB_SPECIAL_CONST_P(errinfo)) {
3035+
rb_bug("exception object was lost during stack unwinding; "
3036+
"a native extension may have run Ruby code containing a rescue clause "
3037+
"between rb_protect() and rb_jump_tag()");
3038+
}
3039+
30283040
for (;;) {
30293041
unsigned int i;
30303042
const struct iseq_catch_table_entry *entry;

0 commit comments

Comments
 (0)