[ruby-core:64848] [ruby-trunk - Bug #10206] garbage symbols crash symbol GC

Issue #10206 has been updated by Eric Wong.


 [e-mail] wrote:
 > At first, Symbol is VALUE and it should be marked.
 > 
 > So that the following code should not be allowed.
 > 
 > ```
 > id = SYM2ID(garbage_sym);
 > ```
 > 
 > In this case, afeter sweeping, garbage_sym becomes freed VALUE.
 > 
 > What happen on it?
 
 Looking at this more, we may run dsymbol_check too late in
 dsymbol_pindown.  I think we must run dsymbol_check immediately after
 looking up dynamic syms from global_symbol.str_id, and not later.
 
 I think this may be a fix (still testing):
 
 --- a/symbol.c
 +++ b/symbol.c
 @@ -458,7 +458,10 @@ dsymbol_pindown(VALUE sym)
  
      if (UNLIKELY(SYMBOL_PINNED_P(sym) == 0)) {
  	VALUE fstr = RSYMBOL(sym)->fstr;
 -	sym = dsymbol_check(sym);
 +
 +	if (UNLIKELY(rb_objspace_garbage_object_p(sym))) {
 +	    rb_bug("attempted to pindown garbage sym");
 +	}
  	FL_SET(sym, SYMBOL_PINNED);
  
  	/* make it permanent object */
 @@ -525,6 +528,9 @@ rb_intern_cstr_without_pindown(const char *name, long len, rb_encoding *enc)
      OBJ_FREEZE(str);
  
      if (st_lookup(global_symbols.str_id, str, &id)) {
 +	if (ID_DYNAMIC_SYM_P((ID)id)) {
 +	    return (ID)dsymbol_check((VALUE)id);
 +	}
  	return (ID)id;
      }

----------------------------------------
Bug #10206: garbage symbols crash symbol GC
https://bugs.ruby-lang.org/issues/10206#change-48717

* Author: Eric Wong
* Status: Open
* Priority: Normal
* Assignee: Koichi Sasada
* Category: core
* Target version: current: 2.2.0
* ruby -v: trunk
* Backport: 2.0.0: UNKNOWN, 2.1: UNKNOWN
----------------------------------------
This is reproducible with just a test loop running for serveral minutes/hours:

	while make test-all TESTS=-j8; do :; done

It looks like SYM2ID/rb_sym2id interacts badly with dsymbol_check
when it encounters garbage objects.

dsymbol_check replaces an invalid object and returns a new object
for the caller, but the original arg for SYM2ID remains usable
to the caller:

        id = SYM2ID(garbage_sym);
        do_something(garbage_sym); /* bad invalid object used */

Changing: rb_sym2id(VALUE) to rb_sym2id(VALUE *)
might solve the issue, but introduces many incompatibilities in existing
code:

        id = rb_sym2id(&garbage_sym);
        do_something(garbage_sym); /* id == garbage_sym, safe to use */

ref: ruby-core thread starting at [ruby-core:64671]
backtraces:
	http://80x24.org/r35240/rb-dump.txt
	http://80x24.org/r35240/gdb-bt.txt





-- 
https://bugs.ruby-lang.org/