Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Abort on system stack overflow during GC #3661

Merged
merged 1 commit into from
Oct 16, 2020
Merged

Conversation

XrXr
Copy link
Member

@XrXr XrXr commented Oct 15, 2020

Buggy native extensions could have mark functions that cause stack
overflow. When a stack overflow happens during GC, Ruby used to recover
by raising an exception, which runs the interpreter. It's not safe to
run the interpreter during GC since the GC is in an inconsistent state.
This could cause object allocation during GC, for example.

Instead of running the interpreter and potentially causing a crash down
the line, fail fast and abort.


I have a script here that illustrates what this is trying to fix. You want to run this in an empty folder.

require 'rbconfig'

File.write("bad-ext.c", <<-EOM)
#include <stdint.h>
#include <stdio.h>
#include "ruby.h"

static void
bad_type_mark(void *ptr)
{
  /* This is a buggy mark function. Causes stack overflow. */
  if ((uintptr_t)ptr == 100) {
    ((char *)ptr)[0] = 0;
    return;
  }
  char arr[1024];
  bad_type_mark(arr);
}

static void
bad_type_free(void *ptr)
{
  /* do nothing */
}

static size_t
bad_type_memsize(const void *ptr)
{
  return 1;
}

static const rb_data_type_t bad_type = {
  "bad_type",
  {
    bad_type_mark,
    bad_type_free,
    bad_type_memsize
  },
  NULL, NULL, RUBY_TYPED_FREE_IMMEDIATELY
};

static VALUE cBadType;
struct bad_type {};

VALUE
make_bad_object(VALUE self)
{
  struct bad_type *res;
  return TypedData_Make_Struct(cBadType, struct bad_type, &bad_type, res);
}


void
Init_bad(void)
{
  cBadType = rb_define_class("Bad", rb_cObject);
  rb_define_global_function("bad_object", make_bad_object, 0);
}
EOM

child = fork do
  require 'mkmf'
  create_makefile('bad')
end
Process.wait(child) if child

`make`

require_relative 'bad'
foo = bad_object
p foo
GC.start

This crashes Ruby 2.7.2 and master with [BUG] object allocation during garbage collection phase when the problem actually lies with the faulty mark function in the C extension.

I found this issue in a gem in the wild. It has a recursive mark function for one of its custom types and sometime causes stack overflow during GC.

Buggy native extensions could have mark functions that cause stack
overflow. When a stack overflow happens during GC, Ruby used to recover
by raising an exception, which runs the interpreter. It's not safe to
run the interpreter during GC since the GC is in an inconsistent state.
This could cause object allocation during GC, for example.

Instead of running the interpreter and potentially causing a crash down
the line, fail fast and abort.
@XrXr XrXr requested review from ko1 and tenderlove October 15, 2020 21:39
Copy link
Contributor

@ko1 ko1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!

@XrXr XrXr merged commit 0d17cdd into ruby:master Oct 16, 2020
@XrXr XrXr deleted the mark-overflow-abort branch October 16, 2020 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants