From da7d7f881bbbad9988a3a2b7bad8f2b72ff06bc6 Mon Sep 17 00:00:00 2001 From: dearblue Date: Sun, 20 Jan 2019 12:00:38 +0900 Subject: [PATCH 01/18] Fix NULL pointer dereference `mrb->nomem_err` when not initialized Add internal functions (not `static`): * `mrb_raise_nomemory()` * `mrb_core_init_abort()` --- src/error.c | 34 +++++++++++++++++++++++++++++----- src/gc.c | 6 ++++-- 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/src/error.c b/src/error.c index 57cdfcfe13..6674fe7ad8 100644 --- a/src/error.c +++ b/src/error.c @@ -240,6 +240,16 @@ mrb_exc_set(mrb_state *mrb, mrb_value exc) } } +static mrb_noreturn void +exc_throw(mrb_state *mrb, mrb_value exc) +{ + if (!mrb->jmp) { + mrb_p(mrb, exc); + abort(); + } + MRB_THROW(mrb->jmp); +} + MRB_API mrb_noreturn void mrb_exc_raise(mrb_state *mrb, mrb_value exc) { @@ -247,11 +257,7 @@ mrb_exc_raise(mrb_state *mrb, mrb_value exc) mrb_raise(mrb, E_TYPE_ERROR, "exception object expected"); } mrb_exc_set(mrb, exc); - if (!mrb->jmp) { - mrb_p(mrb, exc); - abort(); - } - MRB_THROW(mrb->jmp); + exc_throw(mrb, exc); } MRB_API mrb_noreturn void @@ -483,6 +489,24 @@ mrb_no_method_error(mrb_state *mrb, mrb_sym id, mrb_value args, char const* fmt, mrb_exc_raise(mrb, exc); } +mrb_noreturn void +mrb_core_init_abort(mrb_state *mrb) +{ + mrb->exc = NULL; + exc_throw(mrb, mrb_nil_value()); +} + +mrb_noreturn void +mrb_raise_nomemory(mrb_state *mrb) +{ + if (mrb->nomem_err) { + mrb_exc_raise(mrb, mrb_obj_value(mrb->nomem_err)); + } + else { + mrb_core_init_abort(mrb); + } +} + void mrb_init_exception(mrb_state *mrb) { diff --git a/src/gc.c b/src/gc.c index ec52787e83..c67454f5f8 100644 --- a/src/gc.c +++ b/src/gc.c @@ -198,6 +198,8 @@ gettimeofday_time(void) #define objects(p) ((RVALUE *)p->objects) +mrb_noreturn void mrb_raise_nomemory(mrb_state *mrb); + MRB_API void* mrb_realloc_simple(mrb_state *mrb, void *p, size_t len) { @@ -221,12 +223,12 @@ mrb_realloc(mrb_state *mrb, void *p, size_t len) if (len == 0) return p2; if (p2 == NULL) { if (mrb->gc.out_of_memory) { - mrb_exc_raise(mrb, mrb_obj_value(mrb->nomem_err)); + mrb_raise_nomemory(mrb); /* mrb_panic(mrb); */ } else { mrb->gc.out_of_memory = TRUE; - mrb_exc_raise(mrb, mrb_obj_value(mrb->nomem_err)); + mrb_raise_nomemory(mrb); } } else { From fef1c152ce4e52b9e4a34dc23aca5b02907ac639 Mon Sep 17 00:00:00 2001 From: dearblue Date: Sat, 19 Jan 2019 13:05:09 +0900 Subject: [PATCH 02/18] Fix stack overflow when out of memory As a result of this change, no backtrace information is set for NoMemoryError (`mrb->nomem_err`). Detailes: When generating a backtrace, called `mrb_intern_lit()`, `mrb_str_new_cstr()` and `mrb_obj_iv_set()` function with `exc_debug_info()` function in `src/error.c`. If a `NoMemoryError` exception occurs at this time, the `exc_debug_info()` function will be called again, and in the same way `NoMemoryError` exception raised will result in an infinite loop to occurs stack overflow (and SIGSEGV). --- src/error.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/error.c b/src/error.c index 6674fe7ad8..fe193a9a93 100644 --- a/src/error.c +++ b/src/error.c @@ -200,6 +200,7 @@ exc_debug_info(mrb_state *mrb, struct RObject *exc) mrb_callinfo *ci = mrb->c->ci; mrb_code *pc = ci->pc; + if (!exc || exc == mrb->nomem_err) return; if (mrb_obj_iv_defined(mrb, exc, mrb_intern_lit(mrb, "file"))) return; while (ci >= mrb->c->cibase) { mrb_code *err = ci->err; From 7a0418304ec70764fa215bef3599f5f735222075 Mon Sep 17 00:00:00 2001 From: dearblue Date: Fri, 18 Jan 2019 20:38:27 +0900 Subject: [PATCH 03/18] Fix memory leak for string object when out of memory The `mrb_str_pool()` function has a path to call `malloc()` twice. If occurs `NoMemoryError` exception in second `malloc()`, first `malloc()` pointer is not freed. --- src/state.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/state.c b/src/state.c index e678b37f3d..9405d7efb6 100644 --- a/src/state.c +++ b/src/state.c @@ -176,6 +176,8 @@ mrb_irep_free(mrb_state *mrb, mrb_irep *irep) mrb_free(mrb, irep); } +mrb_noreturn void mrb_raise_nomemory(mrb_state *mrb); + mrb_value mrb_str_pool(mrb_state *mrb, mrb_value str) { @@ -214,7 +216,11 @@ mrb_str_pool(mrb_state *mrb, mrb_value str) ns->as.ary[len] = '\0'; } else { - ns->as.heap.ptr = (char *)mrb_malloc(mrb, (size_t)len+1); + ns->as.heap.ptr = (char *)mrb_malloc_simple(mrb, (size_t)len+1); + if (!ns->as.heap.ptr) { + mrb_free(mrb, ns); + mrb_raise_nomemory(mrb); + } ns->as.heap.len = len; ns->as.heap.aux.capa = len; if (ptr) { From 100642750e4d549f2e8050f8d6cabdf8825d4495 Mon Sep 17 00:00:00 2001 From: dearblue Date: Sun, 20 Jan 2019 11:59:02 +0900 Subject: [PATCH 04/18] Protect exception for mruby core initialization --- src/error.c | 20 ++++++++++++++++++++ src/state.c | 37 +++++++++++++++++++++++++++++-------- 2 files changed, 49 insertions(+), 8 deletions(-) diff --git a/src/error.c b/src/error.c index fe193a9a93..cf3d861fde 100644 --- a/src/error.c +++ b/src/error.c @@ -490,6 +490,26 @@ mrb_no_method_error(mrb_state *mrb, mrb_sym id, mrb_value args, char const* fmt, mrb_exc_raise(mrb, exc); } +int +mrb_core_init_protect(mrb_state *mrb, void (*body)(mrb_state *, void *), void *opaque) +{ + struct mrb_jmpbuf *prev_jmp = mrb->jmp; + struct mrb_jmpbuf c_jmp; + int err = 1; + + MRB_TRY(&c_jmp) { + mrb->jmp = &c_jmp; + body(mrb, opaque); + err = 0; + } MRB_CATCH(&c_jmp) { + mrb->exc = NULL; + } MRB_END_EXC(&c_jmp); + + mrb->jmp = prev_jmp; + + return err; +} + mrb_noreturn void mrb_core_init_abort(mrb_state *mrb) { diff --git a/src/state.c b/src/state.c index 9405d7efb6..54a1ebbf1c 100644 --- a/src/state.c +++ b/src/state.c @@ -19,11 +19,25 @@ void mrb_init_mrbgems(mrb_state*); void mrb_gc_init(mrb_state*, mrb_gc *gc); void mrb_gc_destroy(mrb_state*, mrb_gc *gc); +int mrb_core_init_protect(mrb_state *mrb, void (*body)(mrb_state *, void *), void *opaque); + +static void +init_gc_and_core(mrb_state *mrb, void *opaque) +{ + static const struct mrb_context mrb_context_zero = { 0 }; + + mrb_gc_init(mrb, &mrb->gc); + mrb->c = (struct mrb_context*)mrb_malloc(mrb, sizeof(struct mrb_context)); + *mrb->c = mrb_context_zero; + mrb->root_c = mrb->c; + + mrb_init_core(mrb); +} + MRB_API mrb_state* mrb_open_core(mrb_allocf f, void *ud) { static const mrb_state mrb_state_zero = { 0 }; - static const struct mrb_context mrb_context_zero = { 0 }; mrb_state *mrb; mrb = (mrb_state *)(f)(NULL, NULL, sizeof(mrb_state), ud); @@ -34,12 +48,10 @@ mrb_open_core(mrb_allocf f, void *ud) mrb->allocf = f; mrb->atexit_stack_len = 0; - mrb_gc_init(mrb, &mrb->gc); - mrb->c = (struct mrb_context*)mrb_malloc(mrb, sizeof(struct mrb_context)); - *mrb->c = mrb_context_zero; - mrb->root_c = mrb->c; - - mrb_init_core(mrb); + if (mrb_core_init_protect(mrb, init_gc_and_core, NULL)) { + mrb_close(mrb); + return NULL; + } #if !defined(MRB_DISABLE_STDIO) && defined(_MSC_VER) && _MSC_VER < 1900 _set_output_format(_TWO_DIGIT_EXPONENT); @@ -100,6 +112,12 @@ mrb_open(void) return mrb; } +static void +init_mrbgems(mrb_state *mrb, void *opaque) +{ + mrb_init_mrbgems(mrb); +} + MRB_API mrb_state* mrb_open_allocf(mrb_allocf f, void *ud) { @@ -110,7 +128,10 @@ mrb_open_allocf(mrb_allocf f, void *ud) } #ifndef DISABLE_GEMS - mrb_init_mrbgems(mrb); + if (mrb_core_init_protect(mrb, init_mrbgems, NULL)) { + mrb_close(mrb); + return NULL; + } mrb_gc_arena_restore(mrb, 0); #endif return mrb; From d9c7b6be6eb54630b64eea5c35be241e551676e5 Mon Sep 17 00:00:00 2001 From: dearblue Date: Sun, 20 Jan 2019 15:25:09 +0900 Subject: [PATCH 05/18] Fix protect exception for print error message --- src/error.c | 10 +++++++++- src/print.c | 30 +++++++++++++++++++++++++++--- 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/src/error.c b/src/error.c index cf3d861fde..6991e61497 100644 --- a/src/error.c +++ b/src/error.c @@ -490,6 +490,8 @@ mrb_no_method_error(mrb_state *mrb, mrb_sym id, mrb_value args, char const* fmt, mrb_exc_raise(mrb, exc); } +void mrb_core_init_printabort(void); + int mrb_core_init_protect(mrb_state *mrb, void (*body)(mrb_state *, void *), void *opaque) { @@ -502,7 +504,13 @@ mrb_core_init_protect(mrb_state *mrb, void (*body)(mrb_state *, void *), void *o body(mrb, opaque); err = 0; } MRB_CATCH(&c_jmp) { - mrb->exc = NULL; + if (mrb->exc) { + mrb_p(mrb, mrb_obj_value(mrb->exc)); + mrb->exc = NULL; + } + else { + mrb_core_init_printabort(); + } } MRB_END_EXC(&c_jmp); mrb->jmp = prev_jmp; diff --git a/src/print.c b/src/print.c index 03b5eadfaf..6b4f9a4908 100644 --- a/src/print.c +++ b/src/print.c @@ -7,24 +7,48 @@ #include #include #include +#include +#include #ifndef MRB_DISABLE_STDIO +static void +printcstr(const char *str, size_t len, FILE *stream) +{ + if (str) { + fwrite(str, len, 1, stream); + putc('\n', stream); + } +} + static void printstr(mrb_value obj, FILE *stream) { if (mrb_string_p(obj)) { - fwrite(RSTRING_PTR(obj), RSTRING_LEN(obj), 1, stream); - putc('\n', stream); + printcstr(RSTRING_PTR(obj), RSTRING_LEN(obj), stream); } } #else +# define printcstr(str, len, stream) (void)0 # define printstr(obj, stream) (void)0 #endif +void +mrb_core_init_printabort(void) +{ + static const char *str = "Failed mruby core initialization"; + printcstr(str, strlen(str), stdout); +} + MRB_API void mrb_p(mrb_state *mrb, mrb_value obj) { - printstr(mrb_inspect(mrb, obj), stdout); + if (mrb_type(obj) == MRB_TT_EXCEPTION && mrb_obj_ptr(obj) == mrb->nomem_err) { + static const char *str = "Out of memory"; + printcstr(str, strlen(str), stdout); + } + else { + printstr(mrb_inspect(mrb, obj), stdout); + } } MRB_API void From a3cfe755ab3e758046c3f4e30938ac8d567ed046 Mon Sep 17 00:00:00 2001 From: dearblue Date: Sat, 19 Jan 2019 10:11:37 +0900 Subject: [PATCH 06/18] Fix NULL pointer dereference `mrb->c` by `mark_context()` --- src/gc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/gc.c b/src/gc.c index c67454f5f8..bf544fc12f 100644 --- a/src/gc.c +++ b/src/gc.c @@ -1277,6 +1277,7 @@ mrb_full_gc(mrb_state *mrb) { mrb_gc *gc = &mrb->gc; + if (!mrb->c) return; if (gc->disabled || gc->iterating) return; GC_INVOKE_TIME_REPORT("mrb_full_gc()"); From 3f8e2b375244f5441e8d62efa13c6e6a9afecb14 Mon Sep 17 00:00:00 2001 From: dearblue Date: Sun, 20 Jan 2019 02:08:13 +0900 Subject: [PATCH 07/18] Fix keep wrong symbol capacity when out of memory --- src/symbol.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/symbol.c b/src/symbol.c index 6b4c7200c9..7885124234 100644 --- a/src/symbol.c +++ b/src/symbol.c @@ -67,9 +67,11 @@ sym_intern(mrb_state *mrb, const char *name, size_t len, mrb_bool lit) /* registering a new symbol */ sym = ++mrb->symidx; if (mrb->symcapa < sym) { - if (mrb->symcapa == 0) mrb->symcapa = 100; - else mrb->symcapa = (size_t)(mrb->symcapa * 6 / 5); - mrb->symtbl = (symbol_name*)mrb_realloc(mrb, mrb->symtbl, sizeof(symbol_name)*(mrb->symcapa+1)); + size_t symcapa = mrb->symcapa; + if (symcapa == 0) symcapa = 100; + else symcapa = (size_t)(symcapa * 6 / 5); + mrb->symtbl = (symbol_name*)mrb_realloc(mrb, mrb->symtbl, sizeof(symbol_name)*(symcapa+1)); + mrb->symcapa = symcapa; } sname = &mrb->symtbl[sym]; sname->len = (uint16_t)len; From 15e67297ff54bc14ef359d6d1e745d760a4a255a Mon Sep 17 00:00:00 2001 From: dearblue Date: Sun, 20 Jan 2019 02:12:24 +0900 Subject: [PATCH 08/18] Fix keep wrong symbol index when out of memory --- src/symbol.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/symbol.c b/src/symbol.c index 7885124234..f91bfeda33 100644 --- a/src/symbol.c +++ b/src/symbol.c @@ -65,7 +65,7 @@ sym_intern(mrb_state *mrb, const char *name, size_t len, mrb_bool lit) } /* registering a new symbol */ - sym = ++mrb->symidx; + sym = mrb->symidx + 1; if (mrb->symcapa < sym) { size_t symcapa = mrb->symcapa; if (symcapa == 0) symcapa = 100; @@ -86,6 +86,7 @@ sym_intern(mrb_state *mrb, const char *name, size_t len, mrb_bool lit) sname->name = (const char*)p; sname->lit = FALSE; } + mrb->symidx = sym; kh_put(n2s, mrb, h, sym); return sym; From 19162dd6c11f0093d0011e7cab83b8f9e84c2c07 Mon Sep 17 00:00:00 2001 From: dearblue Date: Sun, 20 Jan 2019 02:15:07 +0900 Subject: [PATCH 09/18] Fix memory leak for symbol string when out of memory in `kh_put()` --- include/mruby/khash.h | 10 ++++++++-- src/symbol.c | 1 + 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/include/mruby/khash.h b/include/mruby/khash.h index 9c40c6b802..73a5fed0f5 100644 --- a/include/mruby/khash.h +++ b/include/mruby/khash.h @@ -73,6 +73,7 @@ static const uint8_t __m_either[] = {0x03, 0x0c, 0x30, 0xc0}; void kh_clear_##name(mrb_state *mrb, kh_##name##_t *h); \ khint_t kh_get_##name(mrb_state *mrb, kh_##name##_t *h, khkey_t key); \ khint_t kh_put_##name(mrb_state *mrb, kh_##name##_t *h, khkey_t key, int *ret); \ + void kh_put_prepare_##name(mrb_state *mrb, kh_##name##_t *h); \ void kh_resize_##name(mrb_state *mrb, kh_##name##_t *h, khint_t new_n_buckets); \ void kh_del_##name(mrb_state *mrb, kh_##name##_t *h, khint_t x); \ kh_##name##_t *kh_copy_##name(mrb_state *mrb, kh_##name##_t *h); @@ -171,12 +172,16 @@ kh_fill_flags(uint8_t *p, uint8_t c, size_t len) mrb_free(mrb, old_keys); \ } \ } \ - khint_t kh_put_##name(mrb_state *mrb, kh_##name##_t *h, khkey_t key, int *ret) \ + void kh_put_prepare_##name(mrb_state *mrb, kh_##name##_t *h) \ { \ - khint_t k, del_k, step = 0; \ if (h->n_occupied >= khash_upper_bound(h)) { \ kh_resize_##name(mrb, h, h->n_buckets*2); \ } \ + } \ + khint_t kh_put_##name(mrb_state *mrb, kh_##name##_t *h, khkey_t key, int *ret) \ + { \ + khint_t k, del_k, step = 0; \ + kh_put_prepare_##name(mrb, h); \ k = __hash_func(mrb,key) & khash_mask(h); \ del_k = kh_end(h); \ while (!__ac_isempty(h->ed_flags, k)) { \ @@ -239,6 +244,7 @@ kh_fill_flags(uint8_t *p, uint8_t c, size_t len) #define kh_destroy(name, mrb, h) kh_destroy_##name(mrb, h) #define kh_clear(name, mrb, h) kh_clear_##name(mrb, h) #define kh_resize(name, mrb, h, s) kh_resize_##name(mrb, h, s) +#define kh_put_prepare(name, mrb, h) kh_put_prepare_##name(mrb, h) #define kh_put(name, mrb, h, k) kh_put_##name(mrb, h, k, NULL) #define kh_put2(name, mrb, h, k, r) kh_put_##name(mrb, h, k, r) #define kh_get(name, mrb, h, k) kh_get_##name(mrb, h, k) diff --git a/src/symbol.c b/src/symbol.c index f91bfeda33..31794eb35f 100644 --- a/src/symbol.c +++ b/src/symbol.c @@ -73,6 +73,7 @@ sym_intern(mrb_state *mrb, const char *name, size_t len, mrb_bool lit) mrb->symtbl = (symbol_name*)mrb_realloc(mrb, mrb->symtbl, sizeof(symbol_name)*(symcapa+1)); mrb->symcapa = symcapa; } + kh_put_prepare(n2s, mrb, h); sname = &mrb->symtbl[sym]; sname->len = (uint16_t)len; if (lit || mrb_ro_data_p(name)) { From b6214ff8a0a1c73bc9554e39053878ac50bb683f Mon Sep 17 00:00:00 2001 From: dearblue Date: Sat, 19 Jan 2019 12:53:07 +0900 Subject: [PATCH 10/18] Fix memory leak for `khash_t` in `kh_init_size()` when out of memory by `kh_alloc()` --- include/mruby/khash.h | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/include/mruby/khash.h b/include/mruby/khash.h index 73a5fed0f5..bb32436447 100644 --- a/include/mruby/khash.h +++ b/include/mruby/khash.h @@ -96,16 +96,25 @@ kh_fill_flags(uint8_t *p, uint8_t c, size_t len) __hash_equal: hash comparation function */ #define KHASH_DEFINE(name, khkey_t, khval_t, kh_is_map, __hash_func, __hash_equal) \ - void kh_alloc_##name(mrb_state *mrb, kh_##name##_t *h) \ + mrb_noreturn void mrb_raise_nomemory(mrb_state *mrb); \ + int kh_alloc_simple_##name(mrb_state *mrb, kh_##name##_t *h) \ { \ khint_t sz = h->n_buckets; \ size_t len = sizeof(khkey_t) + (kh_is_map ? sizeof(khval_t) : 0); \ - uint8_t *p = (uint8_t*)mrb_malloc(mrb, sizeof(uint8_t)*sz/4+len*sz); \ + uint8_t *p = (uint8_t*)mrb_malloc_simple(mrb, sizeof(uint8_t)*sz/4+len*sz); \ + if (!p) { return 1; } \ h->size = h->n_occupied = 0; \ h->keys = (khkey_t *)p; \ h->vals = kh_is_map ? (khval_t *)(p+sizeof(khkey_t)*sz) : NULL; \ h->ed_flags = p+len*sz; \ kh_fill_flags(h->ed_flags, 0xaa, sz/4); \ + return 0; \ + } \ + void kh_alloc_##name(mrb_state *mrb, kh_##name##_t *h) \ + { \ + if (kh_alloc_simple_##name(mrb, h)) { \ + mrb_raise_nomemory(mrb); \ + } \ } \ kh_##name##_t *kh_init_##name##_size(mrb_state *mrb, khint_t size) { \ kh_##name##_t *h = (kh_##name##_t*)mrb_calloc(mrb, 1, sizeof(kh_##name##_t)); \ @@ -113,7 +122,10 @@ kh_fill_flags(uint8_t *p, uint8_t c, size_t len) size = KHASH_MIN_SIZE; \ khash_power2(size); \ h->n_buckets = size; \ - kh_alloc_##name(mrb, h); \ + if (kh_alloc_simple_##name(mrb, h)) { \ + mrb_free(mrb, h); \ + mrb_raise_nomemory(mrb); \ + } \ return h; \ } \ kh_##name##_t *kh_init_##name(mrb_state *mrb) { \ From e2d6896ebad13694800af49c2625e106b8440ddf Mon Sep 17 00:00:00 2001 From: dearblue Date: Sat, 19 Jan 2019 12:54:19 +0900 Subject: [PATCH 11/18] Fix memory leak for irep when out of memory by `mrb_proc_new()` --- src/proc.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/proc.c b/src/proc.c index dcbeb4f622..acc21fc3ca 100644 --- a/src/proc.c +++ b/src/proc.c @@ -8,6 +8,7 @@ #include #include #include +#include static mrb_code call_iseq[] = { OP_CALL, @@ -287,14 +288,25 @@ proc_lambda(mrb_state *mrb, mrb_value self) return blk; } +static void +tempirep_free(mrb_state *mrb, void *p) +{ + if (p) mrb_irep_free(mrb, (mrb_irep *)p); +} + +static const mrb_data_type tempirep_type = { "temporary irep", tempirep_free }; + void mrb_init_proc(mrb_state *mrb) { struct RProc *p; mrb_method_t m; - mrb_irep *call_irep = (mrb_irep *)mrb_malloc(mrb, sizeof(mrb_irep)); + struct RData *irep_obj = mrb_data_object_alloc(mrb, mrb->object_class, NULL, &tempirep_type); + mrb_irep *call_irep; static const mrb_irep mrb_irep_zero = { 0 }; + call_irep = (mrb_irep *)mrb_malloc(mrb, sizeof(mrb_irep)); + irep_obj->data = call_irep; *call_irep = mrb_irep_zero; call_irep->flags = MRB_ISEQ_NO_FREE; call_irep->iseq = call_iseq; @@ -306,6 +318,7 @@ mrb_init_proc(mrb_state *mrb) mrb_define_method(mrb, mrb->proc_class, "arity", mrb_proc_arity, MRB_ARGS_NONE()); p = mrb_proc_new(mrb, call_irep); + irep_obj->data = NULL; MRB_METHOD_FROM_PROC(m, p); mrb_define_method_raw(mrb, mrb->proc_class, mrb_intern_lit(mrb, "call"), m); mrb_define_method_raw(mrb, mrb->proc_class, mrb_intern_lit(mrb, "[]"), m); From 2531f2631e67e0462749618e2344c733a29238f0 Mon Sep 17 00:00:00 2001 From: dearblue Date: Sun, 20 Jan 2019 10:48:15 +0900 Subject: [PATCH 12/18] Fix NULL pointer dereference when do not finished initializing irep --- src/state.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/state.c b/src/state.c index 54a1ebbf1c..ef662901e7 100644 --- a/src/state.c +++ b/src/state.c @@ -187,9 +187,11 @@ mrb_irep_free(mrb_state *mrb, mrb_irep *irep) } mrb_free(mrb, irep->pool); mrb_free(mrb, irep->syms); - for (i=0; irlen; i++) { - if (irep->reps[i]) - mrb_irep_decref(mrb, irep->reps[i]); + if (irep->reps) { + for (i=0; irlen; i++) { + if (irep->reps[i]) + mrb_irep_decref(mrb, irep->reps[i]); + } } mrb_free(mrb, irep->reps); mrb_free(mrb, irep->lv); From 6b35ebf49a0aa3edb6bbda770ed58681e9c2e6af Mon Sep 17 00:00:00 2001 From: dearblue Date: Sun, 20 Jan 2019 10:55:50 +0900 Subject: [PATCH 13/18] Fix uninitialized pointer dereference when do not finished initializing irep --- src/load.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/load.c b/src/load.c index 55e0845f34..07b36a6c37 100644 --- a/src/load.c +++ b/src/load.c @@ -191,7 +191,7 @@ read_irep_record_1(mrb_state *mrb, const uint8_t *bin, size_t *len, uint8_t flag } } - irep->reps = (mrb_irep**)mrb_malloc(mrb, sizeof(mrb_irep*)*irep->rlen); + irep->reps = (mrb_irep**)mrb_calloc(mrb, irep->rlen, sizeof(mrb_irep*)); diff = src - bin; mrb_assert_int_fit(ptrdiff_t, diff, size_t, SIZE_MAX); From 8b422577e6eae68a28121b88421d937e8707b487 Mon Sep 17 00:00:00 2001 From: dearblue Date: Sun, 20 Jan 2019 10:57:51 +0900 Subject: [PATCH 14/18] Fix memory leak for irep when out of memory --- src/load.c | 42 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/src/load.c b/src/load.c index 07b36a6c37..a32cf12ba8 100644 --- a/src/load.c +++ b/src/load.c @@ -14,6 +14,7 @@ #include #include #include +#include #if SIZE_MAX < UINT32_MAX # error size_t must be at least 32 bits wide @@ -58,6 +59,14 @@ str_to_double(mrb_state *mrb, mrb_value str) } #endif +static void +tempirep_free(mrb_state *mrb, void *p) +{ + if (p) mrb_irep_decref(mrb, (mrb_irep *)p); +} + +static const mrb_data_type tempirep_type = { "temporary irep", tempirep_free }; + static mrb_irep* read_irep_record_1(mrb_state *mrb, const uint8_t *bin, size_t *len, uint8_t flags) { @@ -66,8 +75,11 @@ read_irep_record_1(mrb_state *mrb, const uint8_t *bin, size_t *len, uint8_t flag ptrdiff_t diff; uint16_t tt, pool_data_len, snl; int plen; - int ai = mrb_gc_arena_save(mrb); + struct RData *irep_obj = mrb_data_object_alloc(mrb, mrb->object_class, NULL, &tempirep_type); mrb_irep *irep = mrb_add_irep(mrb); + int ai = mrb_gc_arena_save(mrb); + + irep_obj->data = irep; /* skip record size */ src += sizeof(uint32_t); @@ -197,30 +209,41 @@ read_irep_record_1(mrb_state *mrb, const uint8_t *bin, size_t *len, uint8_t flag mrb_assert_int_fit(ptrdiff_t, diff, size_t, SIZE_MAX); *len = (size_t)diff; + irep_obj->data = NULL; + return irep; } static mrb_irep* read_irep_record(mrb_state *mrb, const uint8_t *bin, size_t *len, uint8_t flags) { + struct RData *irep_obj = mrb_data_object_alloc(mrb, mrb->object_class, NULL, &tempirep_type); + mrb_int ai = mrb_gc_arena_save(mrb); mrb_irep *irep = read_irep_record_1(mrb, bin, len, flags); int i; + mrb_gc_arena_restore(mrb, ai); if (irep == NULL) { return NULL; } + irep_obj->data = irep; + bin += *len; for (i=0; irlen; i++) { size_t rlen; irep->reps[i] = read_irep_record(mrb, bin, &rlen, flags); + mrb_gc_arena_restore(mrb, ai); if (irep->reps[i] == NULL) { return NULL; } bin += rlen; *len += rlen; } + + irep_obj->data = NULL; + return irep; } @@ -554,6 +577,7 @@ static mrb_irep* read_irep(mrb_state *mrb, const uint8_t *bin, uint8_t flags) { int result; + struct RData *irep_obj = NULL; mrb_irep *irep = NULL; const struct rite_section_header *section_header; uint16_t crc; @@ -574,12 +598,15 @@ read_irep(mrb_state *mrb, const uint8_t *bin, uint8_t flags) return NULL; } + irep_obj = mrb_data_object_alloc(mrb, mrb->object_class, NULL, &tempirep_type); + bin += sizeof(struct rite_binary_header); do { section_header = (const struct rite_section_header *)bin; if (memcmp(section_header->section_ident, RITE_SECTION_IREP_IDENT, sizeof(section_header->section_ident)) == 0) { irep = read_section_irep(mrb, bin, flags); if (!irep) return NULL; + irep_obj->data = irep; } else if (memcmp(section_header->section_ident, RITE_SECTION_LINENO_IDENT, sizeof(section_header->section_ident)) == 0) { if (!irep) return NULL; /* corrupted data */ @@ -605,6 +632,8 @@ read_irep(mrb_state *mrb, const uint8_t *bin, uint8_t flags) bin += bin_to_uint32(section_header->section_size); } while (memcmp(section_header->section_ident, RITE_BINARY_EOF, sizeof(section_header->section_ident)) != 0); + irep_obj->data = NULL; + return irep; } @@ -650,7 +679,16 @@ load_irep(mrb_state *mrb, mrb_irep *irep, mrbc_context *c) MRB_API mrb_value mrb_load_irep_cxt(mrb_state *mrb, const uint8_t *bin, mrbc_context *c) { - return load_irep(mrb, mrb_read_irep(mrb, bin), c); + struct RData *irep_obj = mrb_data_object_alloc(mrb, mrb->object_class, NULL, &tempirep_type); + mrb_irep *irep = mrb_read_irep(mrb, bin); + mrb_value ret; + + irep_obj->data = irep; + mrb_irep_incref(mrb, irep); + ret = load_irep(mrb, irep, c); + irep_obj->data = NULL; + mrb_irep_decref(mrb, irep); + return ret; } MRB_API mrb_value From 8e993167dec62a9709d6faacd517729ddcedf4f9 Mon Sep 17 00:00:00 2001 From: dearblue Date: Sun, 20 Jan 2019 11:41:09 +0900 Subject: [PATCH 15/18] Fix memory leak for temporary filenames when out of memory --- src/load.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/load.c b/src/load.c index a32cf12ba8..cd1858010e 100644 --- a/src/load.c +++ b/src/load.c @@ -422,6 +422,7 @@ read_section_debug(mrb_state *mrb, const uint8_t *start, mrb_irep *irep, uint8_t int result; uint16_t filenames_len; mrb_sym *filenames; + mrb_value filenames_obj; bin = start; header = (struct rite_section_debug_header *)bin; @@ -429,7 +430,8 @@ read_section_debug(mrb_state *mrb, const uint8_t *start, mrb_irep *irep, uint8_t filenames_len = bin_to_uint16(bin); bin += sizeof(uint16_t); - filenames = (mrb_sym*)mrb_malloc(mrb, sizeof(mrb_sym) * (size_t)filenames_len); + filenames_obj = mrb_str_new(mrb, NULL, sizeof(mrb_sym) * (size_t)filenames_len); + filenames = (mrb_sym*)RSTRING_PTR(filenames_obj); for (i = 0; i < filenames_len; ++i) { uint16_t f_len = bin_to_uint16(bin); bin += sizeof(uint16_t); @@ -453,7 +455,7 @@ read_section_debug(mrb_state *mrb, const uint8_t *start, mrb_irep *irep, uint8_t } debug_exit: - mrb_free(mrb, filenames); + mrb_str_resize(mrb, filenames_obj, 0); return result; } From 4c5499b88e47cc6012ad7d7379cb6bc74c6a0b60 Mon Sep 17 00:00:00 2001 From: dearblue Date: Sun, 20 Jan 2019 11:42:07 +0900 Subject: [PATCH 16/18] Fix uninitialized pointer dereference for debug section --- src/debug.c | 13 ++++++++----- src/load.c | 10 +++++----- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/debug.c b/src/debug.c index 2949131e42..c05388e518 100644 --- a/src/debug.c +++ b/src/debug.c @@ -205,11 +205,14 @@ mrb_debug_info_free(mrb_state *mrb, mrb_irep_debug_info *d) if (!d) { return; } - for (i = 0; i < d->flen; ++i) { - mrb_assert(d->files[i]); - mrb_free(mrb, d->files[i]->lines.ptr); - mrb_free(mrb, d->files[i]); + if (d->files) { + for (i = 0; i < d->flen; ++i) { + if (d->files[i]) { + mrb_free(mrb, d->files[i]->lines.ptr); + mrb_free(mrb, d->files[i]); + } + } + mrb_free(mrb, d->files); } - mrb_free(mrb, d->files); mrb_free(mrb, d); } diff --git a/src/load.c b/src/load.c index cd1858010e..c80fb019f7 100644 --- a/src/load.c +++ b/src/load.c @@ -327,14 +327,14 @@ read_debug_record(mrb_state *mrb, const uint8_t *start, mrb_irep* irep, size_t * if (irep->debug_info) { return MRB_DUMP_INVALID_IREP; } - irep->debug_info = (mrb_irep_debug_info*)mrb_malloc(mrb, sizeof(mrb_irep_debug_info)); + irep->debug_info = (mrb_irep_debug_info*)mrb_calloc(mrb, 1, sizeof(mrb_irep_debug_info)); irep->debug_info->pc_count = (uint32_t)irep->ilen; record_size = (size_t)bin_to_uint32(bin); bin += sizeof(uint32_t); irep->debug_info->flen = bin_to_uint16(bin); - irep->debug_info->files = (mrb_irep_debug_info_file**)mrb_malloc(mrb, sizeof(mrb_irep_debug_info*) * irep->debug_info->flen); + irep->debug_info->files = (mrb_irep_debug_info_file**)mrb_calloc(mrb, irep->debug_info->flen, sizeof(mrb_irep_debug_info*)); bin += sizeof(uint16_t); for (f_idx = 0; f_idx < irep->debug_info->flen; ++f_idx) { @@ -342,7 +342,7 @@ read_debug_record(mrb_state *mrb, const uint8_t *start, mrb_irep* irep, size_t * uint16_t filename_idx; mrb_int len; - file = (mrb_irep_debug_info_file *)mrb_malloc(mrb, sizeof(*file)); + file = (mrb_irep_debug_info_file *)mrb_calloc(mrb, 1, sizeof(*file)); irep->debug_info->files[f_idx] = file; file->start_pos = bin_to_uint32(bin); @@ -374,8 +374,8 @@ read_debug_record(mrb_state *mrb, const uint8_t *start, mrb_irep* irep, size_t * case mrb_debug_line_flat_map: { uint32_t l; - file->lines.flat_map = (mrb_irep_debug_info_line*)mrb_malloc( - mrb, sizeof(mrb_irep_debug_info_line) * (size_t)(file->line_entry_count)); + file->lines.flat_map = (mrb_irep_debug_info_line*)mrb_calloc( + mrb, (size_t)(file->line_entry_count), sizeof(mrb_irep_debug_info_line)); for (l = 0; l < file->line_entry_count; ++l) { file->lines.flat_map[l].start_pos = bin_to_uint32(bin); bin += sizeof(uint32_t); From 52e3d5d8585daf86af3ed12db5ab0efefbc9b956 Mon Sep 17 00:00:00 2001 From: dearblue Date: Sat, 19 Jan 2019 21:55:36 +0900 Subject: [PATCH 17/18] Fix memory leak for temporary symbols when out of memory --- src/load.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/load.c b/src/load.c index c80fb019f7..a965e709cf 100644 --- a/src/load.c +++ b/src/load.c @@ -513,6 +513,7 @@ read_section_lv(mrb_state *mrb, const uint8_t *start, mrb_irep *irep, uint8_t fl int result; uint32_t syms_len; mrb_sym *syms; + mrb_value syms_obj; mrb_sym (*intern_func)(mrb_state*, const char*, size_t) = (flags & FLAG_SRC_MALLOC)? mrb_intern : mrb_intern_static; @@ -522,7 +523,8 @@ read_section_lv(mrb_state *mrb, const uint8_t *start, mrb_irep *irep, uint8_t fl syms_len = bin_to_uint32(bin); bin += sizeof(uint32_t); - syms = (mrb_sym*)mrb_malloc(mrb, sizeof(mrb_sym) * (size_t)syms_len); + syms_obj = mrb_str_new(mrb, NULL, sizeof(mrb_sym) * (size_t)syms_len); + syms = (mrb_sym*)RSTRING_PTR(syms_obj); for (i = 0; i < syms_len; ++i) { uint16_t const str_len = bin_to_uint16(bin); bin += sizeof(uint16_t); @@ -542,7 +544,7 @@ read_section_lv(mrb_state *mrb, const uint8_t *start, mrb_irep *irep, uint8_t fl } lv_exit: - mrb_free(mrb, syms); + mrb_str_resize(mrb, syms_obj, 0); return result; } From b178914b111dda79a8f36ec4eb3e9d37b76f982e Mon Sep 17 00:00:00 2001 From: dearblue Date: Sat, 19 Jan 2019 22:22:44 +0900 Subject: [PATCH 18/18] Fix invalid pointer free inside other heap's block 1. `e = mrb_obj_alloc(...)` 2. `e->stack = mrb->c->stack` (`mrb->c->stack` is anywhere in the range `stbase...stend`) 3. And raised exception by `mrb_malloc()`! 4. `mrb_free(e->stack)` by GC part (wrong free) --- src/proc.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/proc.c b/src/proc.c index acc21fc3ca..87855be6d0 100644 --- a/src/proc.c +++ b/src/proc.c @@ -120,7 +120,14 @@ mrb_proc_new_cfunc_with_env(mrb_state *mrb, mrb_func_t func, mrb_int argc, const p->flags |= MRB_PROC_ENVSET; mrb_field_write_barrier(mrb, (struct RBasic*)p, (struct RBasic*)e); MRB_ENV_UNSHARE_STACK(e); + + /* NOTE: Prevents keeping invalid addresses when NoMemoryError is raised from `mrb_malloc()`. */ + e->stack = NULL; + MRB_ENV_SET_STACK_LEN(e, 0); + e->stack = (mrb_value*)mrb_malloc(mrb, sizeof(mrb_value) * argc); + MRB_ENV_SET_STACK_LEN(e, argc); + if (argv) { for (i = 0; i < argc; ++i) { e->stack[i] = argv[i];