1
0
Fork 0
mirror of https://github.com/ruby/ruby.git synced 2022-11-09 12:17:21 -05:00

str_duplicate: Don't share with a frozen shared string

This is a follow up for 3f9562015e.
Before this commit, it was possible to create a shared string which
shares with another shared string by passing a frozen shared string
to `str_duplicate`.

Such string looks like:

```
 --------                    -----------------
 | root | ------ owns -----> | root's buffer |
 --------                    -----------------
     ^                             ^   ^
 -----------                       |   |
 | shared1 | ------ references -----   |
 -----------                           |
     ^                                 |
 -----------                           |
 | shared2 | ------ references ---------
 -----------
```

This is bad news because `rb_fstring(shared2)` can make `shared1`
independent, which severs the reference from `shared1` to `root`:

```c
/* from fstr_update_callback() */
str = str_new_frozen(rb_cString, shared2);  /* can return shared1 */
if (STR_SHARED_P(str)) { /* shared1 is also a shared string */
    str_make_independent(str);  /* no frozen check */
}
```

If `shared1` was the only reference to `root`, then `root` can be
reclaimed by the GC, leaving `shared2` in a corrupted state:

```
 -----------                         --------------------
 | shared1 | -------- owns --------> | shared1's buffer |
 -----------                         --------------------
      ^
      |
 -----------                         -------------------------
 | shared2 | ------ references ----> | root's buffer (freed) |
 -----------                         -------------------------
```

Here is a reproduction script for the situation this commit fixes.

```ruby
a = ('a' * 24).strip.freeze.strip
-a
p a
4.times { GC.start }
p a
```

 - string.c (str_duplicate): always share with the root string when
   the original is a shared string.
 - test_rb_str_dup.rb: specifically test `rb_str_dup` to make
   sure it does not try to share with a shared string.

[Bug #15792]

Closes: https://github.com/ruby/ruby/pull/2159
This commit is contained in:
Alan Wu 2019-05-08 09:44:41 -04:00 committed by Nobuyoshi Nakada
parent d802698d3e
commit c06ddfee87
No known key found for this signature in database
GPG key ID: 4BC7D6DF58D8DF60
3 changed files with 58 additions and 9 deletions

View file

@ -0,0 +1,35 @@
#include "ruby.h"
VALUE rb_str_dup(VALUE str);
static VALUE
bug_rb_str_dup(VALUE self, VALUE str)
{
rb_check_type(str, T_STRING);
return rb_str_dup(str);
}
static VALUE
bug_shared_string_p(VALUE self, VALUE str)
{
rb_check_type(str, T_STRING);
return RB_FL_TEST(str, RUBY_ELTS_SHARED) && RB_FL_TEST(str, RSTRING_NOEMBED) ? Qtrue : Qfalse;
}
static VALUE
bug_sharing_with_shared_p(VALUE self, VALUE str)
{
rb_check_type(str, T_STRING);
if (bug_shared_string_p(self, str)) {
return bug_shared_string_p(self, RSTRING(str)->as.heap.aux.shared);
}
return Qfalse;
}
void
Init_string_rb_str_dup(VALUE klass)
{
rb_define_singleton_method(klass, "rb_str_dup", bug_rb_str_dup, 1);
rb_define_singleton_method(klass, "shared_string?", bug_shared_string_p, 1);
rb_define_singleton_method(klass, "sharing_with_shared?", bug_sharing_with_shared_p, 1);
}

View file

@ -1504,15 +1504,13 @@ str_duplicate(VALUE klass, VALUE str)
MEMCPY(RSTRING(dup)->as.ary, RSTRING(str)->as.ary, MEMCPY(RSTRING(dup)->as.ary, RSTRING(str)->as.ary,
char, embed_size); char, embed_size);
if (flags & STR_NOEMBED) { if (flags & STR_NOEMBED) {
if (UNLIKELY(!(flags & FL_FREEZE))) { if (FL_TEST_RAW(str, STR_SHARED)) {
if (FL_TEST_RAW(str, STR_SHARED)) { str = RSTRING(str)->as.heap.aux.shared;
str = RSTRING(str)->as.heap.aux.shared; }
} else if (UNLIKELY(!(flags & FL_FREEZE))) {
else { str = str_new_frozen(klass, str);
str = str_new_frozen(klass, str); FL_SET_RAW(str, flags & FL_TAINT);
FL_SET_RAW(str, flags & FL_TAINT); flags = FL_TEST_RAW(str, flag_mask);
flags = FL_TEST_RAW(str, flag_mask);
}
} }
if (flags & STR_NOEMBED) { if (flags & STR_NOEMBED) {
RB_OBJ_WRITE(dup, &RSTRING(dup)->as.heap.aux.shared, str); RB_OBJ_WRITE(dup, &RSTRING(dup)->as.heap.aux.shared, str);

View file

@ -0,0 +1,16 @@
require 'test/unit'
require '-test-/string'
class Test_RbStrDup < Test::Unit::TestCase
def test_nested_shared_non_frozen
str = Bug::String.rb_str_dup(Bug::String.rb_str_dup("a" * 50))
assert_send([Bug::String, :shared_string?, str])
assert_not_send([Bug::String, :sharing_with_shared?, str], '[Bug #15792]')
end
def test_nested_shared_frozen
str = Bug::String.rb_str_dup(Bug::String.rb_str_dup("a" * 50).freeze)
assert_send([Bug::String, :shared_string?, str])
assert_not_send([Bug::String, :sharing_with_shared?, str], '[Bug #15792]')
end
end