From 9543cfa7ee5ff983aefc155537b9b933459aa11d Mon Sep 17 00:00:00 2001 From: dearblue Date: Wed, 31 Jan 2024 22:17:16 +0900 Subject: [PATCH] Fixed use-after-free by backtrace object The `MRB_TT_BACKTRACE` object has been added for the purpose. Previously, "use-after-free" could occur because the reference count in `backtrace_location::irep` was not incremented. fixed #6160 --- include/mruby/internal.h | 12 ++++++ include/mruby/value.h | 3 +- mrbgems/mruby-os-memsize/src/memsize.c | 4 ++ src/backtrace.c | 60 ++++++++++++-------------- src/gc.c | 19 ++++++++ 5 files changed, 64 insertions(+), 34 deletions(-) diff --git a/include/mruby/internal.h b/include/mruby/internal.h index 37c6fa1f0..11b15d057 100644 --- a/include/mruby/internal.h +++ b/include/mruby/internal.h @@ -61,6 +61,18 @@ mrb_value mrb_exc_mesg_get(mrb_state *mrb, struct RException *exc); mrb_value mrb_f_raise(mrb_state*, mrb_value); mrb_value mrb_make_exception(mrb_state *mrb, mrb_value exc, mrb_value mesg); +struct RBacktrace { + MRB_OBJECT_HEADER; + size_t len; + struct mrb_backtrace_location *locations; +}; + +struct mrb_backtrace_location { + mrb_sym method_id; + int32_t idx; + const struct RProc *proc; +}; + /* gc */ void mrb_gc_mark_mt(mrb_state*, struct RClass*); size_t mrb_gc_mark_mt_size(mrb_state*, struct RClass*); diff --git a/include/mruby/value.h b/include/mruby/value.h index 71cdaca35..16da2cbd4 100644 --- a/include/mruby/value.h +++ b/include/mruby/value.h @@ -159,7 +159,8 @@ static const unsigned int IEEE754_INFINITY_BITS_SINGLE = 0x7F800000; f(MRB_TT_BREAK, struct RBreak, "break") \ f(MRB_TT_COMPLEX, struct RComplex, "Complex") \ f(MRB_TT_RATIONAL, struct RRational, "Rational") \ - f(MRB_TT_BIGINT, struct RBigint, "Integer") + f(MRB_TT_BIGINT, struct RBigint, "Integer") \ + f(MRB_TT_BACKTRACE, struct RBacktrace, "backtrace") enum mrb_vtype { #define MRB_VTYPE_DEFINE(tt, type, name) tt, diff --git a/mrbgems/mruby-os-memsize/src/memsize.c b/mrbgems/mruby-os-memsize/src/memsize.c index 10f77beea..8ffd313ed 100644 --- a/mrbgems/mruby-os-memsize/src/memsize.c +++ b/mrbgems/mruby-os-memsize/src/memsize.c @@ -1,4 +1,5 @@ #include +#include #include #include #include @@ -167,6 +168,9 @@ os_memsize_of_object(mrb_state* mrb, mrb_value obj) case MRB_TT_ISTRUCT: size += mrb_objspace_page_slot_size(); break; + case MRB_TT_BACKTRACE: + size += ((struct RBacktrace*)mrb_obj_ptr(obj))->len * sizeof(struct mrb_backtrace_location); + break; /* zero heap size types. * immediate VM stack values, contained within mrb_state, or on C stack */ case MRB_TT_TRUE: diff --git a/src/backtrace.c b/src/backtrace.c index 816b0e6db..ebdfb6f29 100644 --- a/src/backtrace.c +++ b/src/backtrace.c @@ -17,15 +17,7 @@ #include #include -struct backtrace_location { - mrb_sym method_id; - int32_t idx; - const mrb_irep *irep; -}; - -typedef void (*each_backtrace_func)(mrb_state*, const struct backtrace_location*, void*); - -static const mrb_data_type bt_type = { "Backtrace", mrb_free }; +typedef void (*each_backtrace_func)(mrb_state*, const struct mrb_backtrace_location*, void*); static uint32_t each_backtrace(mrb_state *mrb, ptrdiff_t ciidx, each_backtrace_func func, void *data) @@ -33,7 +25,7 @@ each_backtrace(mrb_state *mrb, ptrdiff_t ciidx, each_backtrace_func func, void * uint32_t n = 0; for (ptrdiff_t i=ciidx; i >= 0; i--) { - struct backtrace_location loc; + struct mrb_backtrace_location loc; mrb_callinfo *ci; const mrb_code *pc; @@ -41,22 +33,22 @@ each_backtrace(mrb_state *mrb, ptrdiff_t ciidx, each_backtrace_func func, void * if (!ci->proc || MRB_PROC_CFUNC_P(ci->proc)) { if (!ci->mid) continue; - loc.irep = NULL; + loc.proc = NULL; } else { - loc.irep = ci->proc->body.irep; - if (!loc.irep) continue; - if (!loc.irep->debug_info) continue; + loc.proc = ci->proc; + if (!loc.proc->body.irep) continue; + if (!loc.proc->body.irep->debug_info) continue; if (mrb->c->cibase[i].pc) { pc = &mrb->c->cibase[i].pc[-1]; } else { continue; } - loc.idx = (uint32_t)(pc - loc.irep->iseq); + loc.idx = (uint32_t)(pc - loc.proc->body.irep->iseq); } loc.method_id = ci->mid; - if (loc.irep == NULL) { + if (loc.proc == NULL) { for (ptrdiff_t j=i-1; j >= 0; j--) { ci = &mrb->c->cibase[j]; @@ -74,8 +66,8 @@ each_backtrace(mrb_state *mrb, ptrdiff_t ciidx, each_backtrace_func func, void * continue; } - loc.irep = irep; - loc.idx = (uint32_t)(pc - loc.irep->iseq); + loc.proc = ci->proc; + loc.idx = (uint32_t)(pc - irep->iseq); break; } } @@ -87,11 +79,11 @@ each_backtrace(mrb_state *mrb, ptrdiff_t ciidx, each_backtrace_func func, void * static void pack_backtrace_i(mrb_state *mrb, - const struct backtrace_location *loc, + const struct mrb_backtrace_location *loc, void *data) { - struct backtrace_location **pptr = (struct backtrace_location**)data; - struct backtrace_location *ptr = *pptr; + struct mrb_backtrace_location **pptr = (struct mrb_backtrace_location**)data; + struct mrb_backtrace_location *ptr = *pptr; *ptr = *loc; *pptr = ptr+1; @@ -100,7 +92,7 @@ pack_backtrace_i(mrb_state *mrb, static struct RObject* packed_backtrace(mrb_state *mrb) { - struct RData *backtrace; + struct RBacktrace *backtrace; ptrdiff_t ciidx = mrb->c->ci - mrb->c->cibase; if (ciidx >= mrb->c->ciend - mrb->c->cibase) @@ -108,16 +100,16 @@ packed_backtrace(mrb_state *mrb) /* count the number of backtraces */ int len = each_backtrace(mrb, ciidx, NULL, NULL); - backtrace = mrb_data_object_alloc(mrb, NULL, NULL, &bt_type); + backtrace = MRB_OBJ_ALLOC(mrb, MRB_TT_BACKTRACE, NULL); if (len > 0) { - void *ptr = mrb_malloc(mrb, len * sizeof(struct backtrace_location)); - backtrace->data = ptr; - backtrace->flags = len; + void *ptr = mrb_malloc(mrb, len * sizeof(struct mrb_backtrace_location)); + backtrace->locations = (struct mrb_backtrace_location*)ptr; + backtrace->len = len; each_backtrace(mrb, ciidx, pack_backtrace_i, &ptr); } else { - backtrace->data = NULL; - backtrace->flags = 0; + backtrace->locations = NULL; + backtrace->len = 0; } return (struct RObject*)backtrace; } @@ -146,7 +138,7 @@ mrb_keep_backtrace(mrb_state *mrb, mrb_value exc) static struct RObject* mrb_unpack_backtrace(mrb_state *mrb, struct RObject *backtrace) { - const struct backtrace_location *bt; + const struct mrb_backtrace_location *bt; mrb_int n, i; int ai; @@ -155,19 +147,21 @@ mrb_unpack_backtrace(mrb_state *mrb, struct RObject *backtrace) return mrb_obj_ptr(mrb_ary_new_capa(mrb, 0)); } if (backtrace->tt == MRB_TT_ARRAY) return backtrace; - bt = (struct backtrace_location*)mrb_data_check_get_ptr(mrb, mrb_obj_value(backtrace), &bt_type); + mrb_assert(backtrace->tt == MRB_TT_BACKTRACE); + struct RBacktrace *btobj = (struct RBacktrace*)backtrace; + bt = btobj->locations; if (bt == NULL) goto empty_backtrace; - n = (mrb_int)backtrace->flags; + n = (mrb_int)btobj->len; if (n == 0) goto empty_backtrace; backtrace = mrb_obj_ptr(mrb_ary_new_capa(mrb, n)); ai = mrb_gc_arena_save(mrb); for (i = 0; i < n; i++) { - const struct backtrace_location *entry = &bt[i]; + const struct mrb_backtrace_location *entry = &bt[i]; mrb_value btline; int32_t lineno; const char *filename; - if (!mrb_debug_get_position(mrb, entry->irep, entry->idx, &lineno, &filename)) { + if (!entry->proc || !mrb_debug_get_position(mrb, entry->proc->body.irep, entry->idx, &lineno, &filename)) { btline = mrb_str_new_lit(mrb, "(unknown):0"); } else if (lineno != -1) {//debug info was available diff --git a/src/gc.c b/src/gc.c index 39f63c62b..685f2b055 100644 --- a/src/gc.c +++ b/src/gc.c @@ -688,6 +688,15 @@ gc_mark_children(mrb_state *mrb, mrb_gc *gc, struct RBasic *obj) mrb_gc_mark(mrb, (struct RBasic*)((struct RException*)obj)->backtrace); break; + case MRB_TT_BACKTRACE: + { + struct RBacktrace *bt = (struct RBacktrace*)obj; + for (size_t i = 0; i < bt->len; i++) { + mrb_gc_mark(mrb, (struct RBasic*)bt->locations[i].proc); + } + } + break; + default: break; } @@ -823,6 +832,12 @@ obj_free(mrb_state *mrb, struct RBasic *obj, int end) break; #endif + case MRB_TT_BACKTRACE: + { + struct RBacktrace *bt = (struct RBacktrace*)obj; + mrb_free(mrb, bt->locations); + } + default: break; } @@ -969,6 +984,10 @@ gc_gray_counts(mrb_state *mrb, mrb_gc *gc, struct RBasic *obj) } break; + case MRB_TT_BACKTRACE: + children += ((struct RBacktrace*)obj)->len; + break; + default: break; }