Skip to content

Commit

Permalink
Fix SEGV on re-raising NoMemoryError
Browse files Browse the repository at this point in the history
Think about the following Ruby script:

segv.rb:

    begin
      lambda do
        lambda do
          "x" * 1000 # NoMemoryError
        end.call
      end.call
    rescue
      raise
    end

If memory can't allocate after `"x" * 1000`, mruby crashes.

Because L_RAISE: block in mrb_vm_exec() calls mrb_env_unshare() via
cipop() and mrb_env_unshare() uses allocated memory without NULL check:

L_RAISE: block:

    L_RAISE:
      // ...
      while (ci[0].ridx == ci[-1].ridx) {
        cipop(mrb);
        // ...
      }

cipop():

    static void
    cipop(mrb_state *mrb)
    {
      struct mrb_context *c = mrb->c;

      if (c->ci->env) {
        mrb_env_unshare(mrb, c->ci->env);
      }

      c->ci--;
    }

mrb_env_unshare():

    MRB_API void
    mrb_env_unshare(mrb_state *mrb, struct REnv *e)
    {
      size_t len = (size_t)MRB_ENV_STACK_LEN(e);
      // p is NULL in this case
      mrb_value *p = (mrb_value *)mrb_malloc(mrb, sizeof(mrb_value)*len);

      MRB_ENV_UNSHARE_STACK(e);
      if (len > 0) {
        stack_copy(p, e->stack, len); // p is NULL but used. It causes SEGV.
      }
      e->stack = p;
      mrb_write_barrier(mrb, (struct RBasic *)e);
    }

To solve the SEGV, this change always raises NoMemoryError even when
realloc() is failed after the first NoMemoryError in
mrb_realloc(). mrb_unv_unshare() doesn't need to check NULL with this
change.

But it causes infinite loop in the following while:

    L_RAISE:
      // ...
      while (ci[0].ridx == ci[-1].ridx) {
        cipop(mrb);
        // ...
      }

Because cipop() never pops ci.

This change includes cipop() change. The change pops ci even when
mrb_unv_unshare() is failed by NoMemoryError.

This case can be reproduced by the following program:

    #include <stdlib.h>
    #include <mruby.h>
    #include <mruby/compile.h>

    static void *
    allocf(mrb_state *mrb, void *ptr, size_t size, void *ud)
    {
      static mrb_bool always_fail = FALSE;

      if (size == 1001) {
        always_fail = TRUE;
      }
      if (always_fail) {
        return NULL;
      }

      if (size == 0) {
        free(ptr);
        return NULL;
      } else {
        return realloc(ptr, size);
      }
    }

    int
    main(int argc, char **argv)
    {
      mrb_state *mrb;
      mrbc_context *c;
      FILE *file;

      mrb = mrb_open_allocf(allocf, NULL);
      c = mrbc_context_new(mrb);
      file = fopen(argv[1], "r");
      mrb_load_file_cxt(mrb, file, c);
      fclose(file);
      mrbc_context_free(mrb, c);
      mrb_close(mrb);

      return EXIT_SUCCESS;
    }

Try the following command lines:

    % cc -I include -L build/host/lib -O0 -g3 -o no-memory no-memory.c -lmruby -lm
    % ./no-memory segv.rb
  • Loading branch information
kou committed Jan 19, 2016
1 parent 70d24a9 commit 1d84b32
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 4 deletions.
1 change: 1 addition & 0 deletions src/gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ mrb_realloc(mrb_state *mrb, void *p, size_t len)
p2 = mrb_realloc_simple(mrb, p, len);
if (!p2 && len) {
if (mrb->gc.out_of_memory) {
mrb_exc_raise(mrb, mrb_obj_value(mrb->nomem_err));
/* mrb_panic(mrb); */
}
else {
Expand Down
9 changes: 5 additions & 4 deletions src/vm.c
Original file line number Diff line number Diff line change
Expand Up @@ -256,12 +256,13 @@ static void
cipop(mrb_state *mrb)
{
struct mrb_context *c = mrb->c;

if (c->ci->env) {
mrb_env_unshare(mrb, c->ci->env);
}
struct REnv *env = c->ci->env;

c->ci--;

if (env) {
mrb_env_unshare(mrb, env);
}
}

void mrb_exc_set(mrb_state *mrb, mrb_value exc);
Expand Down

0 comments on commit 1d84b32

Please sign in to comment.