tl;dr

If you enjoy this article, subscribe (via RSS or e-mail) and follow me on twitter.

This post is going to explain a serious design flaw of the object system used in MRI/REE/YARV. This flaw causes seemingly random segfaults and other hard to track corruption. One popular incarnation of this bug is the “rake aborted! not in gzip format.”

theme song

This blog post was inspired by one of my favorite Papoose verses. If you don’t listen to this while reading, you probably won’t understand what I’m talking about: get in the zone.

rake aborted! not in gzip format

[BUG] Segmentation fault



If you’ve seen either of these error messages you are hitting a fundamental flaw of the object model in MRI/YARV. An example of a fix for a single instance of this bug can be seen in this patch. Let’s examine this specific patch so that we can gain some understanding of the general case.

FACT: What you are about to read is absolutely not a compiler bug.

A small, but important piece of background information

The amd64 ABI states that some registers are caller saved, while others are callee saved. In particular, the register rax is caller saved. The callee will overwrite the value in this register to store its return value for the caller so if the caller cares about what is stored in this register, it must be copied prior to a function call.

stare into the abyss part 1

Let’s look at the C code for gzfile_read_raw_ensure WITHOUT the fix from above:

#define zstream_append_input2(z,v)\ zstream_append_input((z), (Bytef*)RSTRING_PTR(v), RSTRING_LEN(v)) static int gzfile_read_raw_ensure(struct gzfile *gz, int size) { VALUE str; while (NIL_P(gz->z.input) || RSTRING_LEN(gz->z.input) < size) { str = gzfile_read_raw(gz); if (NIL_P(str)) return Qfalse; zstream_append_input2(&gz->z, str); } return Qtrue; }

It looks relatively sane at first glance, but to understand this bug we’ll need to examine the assembly generated for this thing. I’m going to rearrange the assembly a bit to make it easier to follow and add few comments a long the way.

First, the code begins by setting the stage:

push %rbp movslq %esi,%rbp # sign extend "size" into rbp push %rbx mov %rdi,%rbx # rbx = gz sub $0x8,%rsp # make room on the stack for "str"

The above is pretty basic. It is your typical amd64 prologue. After things are all setup, it is time to enter into the while loop in the C code above:

jmp 1180 # JUMP IN to the loop

Next comes the NIL_P(gz->z.input) portion of the while -loop condition:

mov 0x18(%rbx),%rax # rax = gz->z.input cmp $0x4,%rax # in Ruby, nil is represented as 4. je 1190 [gzfile_read_raw_ensure+0x30] # if gz->z.input is nil, enter the loop

Now the RSTRING_LEN(gz->z.input) < size portion:

cmp %rbp,0x10(%rax) # compare size and gz->z.input->len jge 11b0 [gzfile_read_raw_ensure+0x50] # jump out of loop # if gz->z.input->len is >= size

Next comes the call to gzfile_read_raw and the NIL_P(str) check. If this check fails, the code just falls through and exits the loop:

mov %rbx,%rdi # rdi = gz, rdi holds the first argument to a function. callq 1090 [gzfile_read_raw] # call gzfile_read_raw cmp $0x4,%rax # compare return value (%rax) to nil jne 1170 [gzfile_read_raw_ensure+0x10] # if it is NOT nil jump to the good stuff

The return value of gzfile_read_raw_ensure (an address of a ruby object) is stored in rax .

And finally, the good stuff. The call to zstream_append_input :

mov 0x10(%rax),%rdx # RSTRING_LEN(v) as 3rd arg mov 0x18(%rax),%rsi # RSTRING_PTR(v) as 2nd arg mov %rbx,%rdi # set gz->z as the 1st arg callq 10e0 [zstream_append_input] # let it rip

Note that the arguments to zstream_append_input are moved into registers by offsetting from rax and that when the call to zstream_append occurs, the ruby object returned from gzfile_read_raw_ensure is still stored in rax and not written to it's slot on the stack because the extra write is unnecessary.

stare into the abyss part 2

Aright, so the patch changes the zstream_append_input2 macro to this:

#define zstream_append_input2(z,v)\ RB_GC_GUARD(v),\ zstream_append_input((z), (Bytef*)RSTRING_PTR(v), RSTRING_LEN(v))

And, RB_GC_GUARD is define d as:

#define RB_GC_GUARD_PTR(ptr) \ __extension__ ({volatile VALUE *rb_gc_guarded_ptr = (ptr); rb_gc_guarded_ptr;}) #define RB_GC_GUARD(v) (*RB_GC_GUARD_PTR(&(v)))

That code is just a hack to mark the memory location holding v with the volatile type qualifier. This tells the compiler that memory backing v acts in ways that the compiler is too stupid to understand, so the compiler must ensure that reads and writes to this location are not optimized out.

A common usage of this qualifier is for memory mapped registers. Reads from memory mapped registers should not be optimized away since a hardware device may update the value stored at that location. The compiler wouldn't know when these updates could happen so it must make sure to re-read the value from this memory location when it is needed. Similarly, writes to memory mapped registers may modify the state of a hardware device and should not be optimized away.

Most of the code generated with the patch applied is the same as without except for a few slight differences before zstream_append_input is called. Let's take a look:

mov %rax,-0x18(%rbp) # write str to the stack mov -0x18(%rbp),%rax # read the value in str back to rax mov 0x10(%rcx),%rdx # RSTRING_LEN(v) mov 0x18(%rcx),%rsi # RSTRING_PTR(v) mov %rbx,%rdi # z callq 1f60 [_zstream_append_input]

The key difference is that the return value of gz_file_read_raw is written back to it's memory location (which, in this case, happens to be on the stack and is called str ).

the bug

The bug is triggered because:

The address of the ruby object str is stored in a caller saved register, rax . The callee ( zstream_append_input ) does not save the value of rax (it is not required to) and rax is overwritten in the function, leaving no references to the ruby object returned by gzfile_read_raw . The callee ( zstream_append_input ) eventually calls rb_newobj . rb_newobj may trigger a GC run, if there are no available objects on the freelist. The GC run finds the object returned by gzfile_read_raw but sees no references to it and frees the memory associated with it. The freed object is used as it were it were valid, and memory corruption occurs causing the VM to explode.

The patch prevents this bug from happening because:

The address of the ruby object str is stored in a caller saved register, rax . The volatile type qualifier causes the compiler to generate code which writes the return value back into it's memory location on the stack. The callee ( zstream_append_input ) eventually calls rb_newobj . rb_newobj may trigger a GC run, if there are no available objects on the freelist. The GC run finds the object returned by gzfile_read_raw and finds a reference to it and therefore does not free it. Everyone is happy.

The general case

Given valid C code, gcc will generate machine instructions that correctly do what you want. Of course, there are bugs in gcc just like any other piece of software. The problem in this case is not gcc . The problem is that the object and garbage collection implementations in REE/MRI/YARV are not valid C code, so it is not possible for gcc to generate machine instructions that do the right thing. In other words, Ruby's object and GC implementations are breaking their contract with gcc .

The end result is the need for shit like RB_GC_GUARD in REE/MRI/YARV and also in Ruby gems to selectively paper over valid gcc optimizations. Having an API that might cause the Ruby VM to fucking explode unless you proactively mark things with RB_GC_GUARD is not on the path of least resistance toward building a maintainable, safe, and performant system. Very few people out there know that the volatile type qualifier exists, let alone what it does. Essentially, this means that authors of Ruby gems must understand how GC works in the VM to prevent their gems from causing GC to break the universe.

That is fucking beyond stupid.

How to detect this bug class

This could be detected by building a simple static analysis tool. You won't catch 100% of cases, and you will definitely have false positives, but it is better than nothing. Something like this should work:

Build a call digraph of the VM and/or the set of gems you care about. Find all paths leading to the rb_newobj sink. Find all paths which call rb_newobj , but do not save rax prior to making another function call which is also on a path to rb_newobj . The functions found are very likely to be causing corruption. A human will need to examine the found cases to weed out false positives and to fix the code.

If you have found yourself wondering who the fuck would write such a test? it is important for you to note that rtld in Linux does not save the SSE registers (which are supposed to be caller saved) prior to entering the fixup function, however to ensure that such an optimization does not cause the fucking universe to come crashing down, a test ships with the code to run objdump after building the binary. The objdump output is then grepped for any instructions which might modify the SSE registers. As long as no one touches the SSE registers, there is no need to save and restore them.

If Ruby's object and GC subsystems want to prevent the universe from exploding, it must supply an equivalent test to ensure that corruption is impossible.

Conclusion

MRI/YARV/REE are inherently fatally flawed.

I'm never writing another Ruby-related blog post.

I'm not a Ruby programmer.

No comments

I'm taking a page from the book of coda and disabling comments. If you got something to say, write a blog post.

If you enjoyed this article, subscribe (via RSS or e-mail) and follow me on twitter.

References