ruby-changes:58118
From: Nobuyoshi <ko1@a...>
Date: Sat, 5 Oct 2019 03:07:29 +0900 (JST)
Subject: [ruby-changes:58118] cbbe198c89 (master): Fix potential memory leaks by `rb_imemo_tmpbuf_auto_free_pointer`
https://git.ruby-lang.org/ruby.git/commit/?id=cbbe198c89 From cbbe198c89fa25a80ec0a5f0592ea00132eacd01 Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada <nobu@r...> Date: Sat, 5 Oct 2019 02:08:07 +0900 Subject: Fix potential memory leaks by `rb_imemo_tmpbuf_auto_free_pointer` This function has been used wrongly always at first, "allocate a buffer then wrap it with tmpbuf". This order can cause a memory leak, as tmpbuf creation also can raise a NoMemoryError exception. The right order is "create a tmpbuf then allocate&wrap a buffer". So the argument of this function is rather harmful than just useless. TODO: * Rename this function to more proper name, as it is not used "temporary" (function local) purpose. * Allocate and wrap at once safely, like `ALLOCV`. diff --git a/gc.c b/gc.c index c9f26cc..fe01d2b 100644 --- a/gc.c +++ b/gc.c @@ -2133,12 +2133,6 @@ rb_imemo_tmpbuf_new(VALUE v1, VALUE v2, VALUE v3, VALUE v0) https://github.com/ruby/ruby/blob/trunk/gc.c#L2133 } VALUE -rb_imemo_tmpbuf_auto_free_pointer(void *buf) -{ - return rb_imemo_new(imemo_tmpbuf, (VALUE)buf, 0, 0, 0); -} - -VALUE rb_imemo_tmpbuf_auto_free_maybe_mark_buffer(void *buf, size_t cnt) { return rb_imemo_tmpbuf_new((VALUE)buf, 0, (VALUE)cnt, 0); diff --git a/internal.h b/internal.h index 7d24e33..e653f30 100644 --- a/internal.h +++ b/internal.h @@ -1134,6 +1134,8 @@ imemo_type_p(VALUE imemo, enum imemo_type imemo_type) https://github.com/ruby/ruby/blob/trunk/internal.h#L1134 } } +VALUE rb_imemo_new(enum imemo_type type, VALUE v1, VALUE v2, VALUE v3, VALUE v0); + /* FL_USER0 to FL_USER3 is for type */ #define IMEMO_FL_USHIFT (FL_USHIFT + 4) #define IMEMO_FL_USER0 FL_USER4 @@ -1203,13 +1205,19 @@ typedef struct rb_imemo_tmpbuf_struct { https://github.com/ruby/ruby/blob/trunk/internal.h#L1205 size_t cnt; /* buffer size in VALUE */ } rb_imemo_tmpbuf_t; -VALUE rb_imemo_tmpbuf_auto_free_pointer(void *buf); +#define rb_imemo_tmpbuf_auto_free_pointer() rb_imemo_new(imemo_tmpbuf, 0, 0, 0, 0) VALUE rb_imemo_tmpbuf_auto_free_maybe_mark_buffer(void *buf, size_t cnt); rb_imemo_tmpbuf_t *rb_imemo_tmpbuf_parser_heap(void *buf, rb_imemo_tmpbuf_t *old_heap, size_t cnt); #define RB_IMEMO_TMPBUF_PTR(v) \ ((void *)(((const struct rb_imemo_tmpbuf_struct *)(v))->ptr)) +static inline void * +rb_imemo_tmpbuf_set_ptr(VALUE v, void *ptr) +{ + return ((rb_imemo_tmpbuf_t *)v)->ptr = ptr; +} + static inline VALUE rb_imemo_tmpbuf_auto_free_pointer_new_from_an_RString(VALUE str) { @@ -1221,7 +1229,7 @@ rb_imemo_tmpbuf_auto_free_pointer_new_from_an_RString(VALUE str) https://github.com/ruby/ruby/blob/trunk/internal.h#L1229 SafeStringValue(str); /* create tmpbuf to keep the pointer before xmalloc */ - imemo = rb_imemo_tmpbuf_auto_free_pointer(NULL); + imemo = rb_imemo_tmpbuf_auto_free_pointer(); tmpbuf = (rb_imemo_tmpbuf_t *)imemo; len = RSTRING_LEN(str); src = RSTRING_PTR(str); diff --git a/parse.y b/parse.y index 1a77332..287704e 100644 --- a/parse.y +++ b/parse.y @@ -481,7 +481,7 @@ static NODE *symbol_append(struct parser_params *p, NODE *symbols, NODE *symbol) https://github.com/ruby/ruby/blob/trunk/parse.y#L481 static NODE *match_op(struct parser_params*,NODE*,NODE*,const YYLTYPE*,const YYLTYPE*); -static ID *local_tbl(struct parser_params*, VALUE *tmp); +static ID *local_tbl(struct parser_params*); static VALUE reg_compile(struct parser_params*, VALUE, int); static void reg_fragment_setenc(struct parser_params*, VALUE, int); @@ -779,9 +779,9 @@ new_array_pattern_tail(struct parser_params *p, VALUE pre_args, VALUE has_rest, https://github.com/ruby/ruby/blob/trunk/parse.y#L779 rest_arg = Qnil; } + VALUE tmpbuf = rb_imemo_tmpbuf_auto_free_pointer(); apinfo = ZALLOC(struct rb_ary_pattern_info); - /* VALUE tmpbuf = rb_imemo_tmpbuf_auto_free_pointer(apinfo); */ - VALUE tmpbuf = rb_imemo_new(imemo_tmpbuf, (VALUE)apinfo, 0, 0, 0); + rb_imemo_tmpbuf_set_ptr(tmpbuf, apinfo); apinfo->imemo = rb_ary_new_from_args(4, pre_args, rest_arg, post_args, tmpbuf); t = rb_node_newnode(NODE_ARYPTN, Qnil, Qnil, (VALUE)apinfo, &NULL_LOC); @@ -2830,8 +2830,9 @@ primary : literal https://github.com/ruby/ruby/blob/trunk/parse.y#L2830 ID id = internal_id(p); NODE *m = NEW_ARGS_AUX(0, 0, &NULL_LOC); NODE *args, *scope, *internal_var = NEW_DVAR(id, &@2); + VALUE tmpbuf = rb_imemo_tmpbuf_auto_free_pointer(); ID *tbl = ALLOC_N(ID, 3); - VALUE tmpbuf = rb_imemo_tmpbuf_auto_free_pointer(tbl); + rb_imemo_tmpbuf_set_ptr(tmpbuf, tbl); tbl[0] = 1 /* length of local var table */; tbl[1] = id /* internal id */; tbl[2] = tmpbuf; @@ -11215,11 +11216,10 @@ static NODE* https://github.com/ruby/ruby/blob/trunk/parse.y#L11216 new_args_tail(struct parser_params *p, NODE *kw_args, ID kw_rest_arg, ID block, const YYLTYPE *loc) { int saved_line = p->ruby_sourceline; - struct rb_args_info *args; NODE *node; - - args = ZALLOC(struct rb_args_info); - VALUE tmpbuf = rb_imemo_tmpbuf_auto_free_pointer(args); + VALUE tmpbuf = rb_imemo_tmpbuf_auto_free_pointer(); + struct rb_args_info *args = ZALLOC(struct rb_args_info); + rb_imemo_tmpbuf_set_ptr(tmpbuf, args); args->imemo = tmpbuf; node = NEW_NODE(NODE_ARGS, 0, 0, args, &NULL_LOC); RB_OBJ_WRITTEN(p->ast, Qnil, tmpbuf); @@ -11309,11 +11309,10 @@ static NODE* https://github.com/ruby/ruby/blob/trunk/parse.y#L11309 new_array_pattern_tail(struct parser_params *p, NODE *pre_args, int has_rest, ID rest_arg, NODE *post_args, const YYLTYPE *loc) { int saved_line = p->ruby_sourceline; - struct rb_ary_pattern_info *apinfo; NODE *node; - - apinfo = ZALLOC(struct rb_ary_pattern_info); - VALUE tmpbuf = rb_imemo_tmpbuf_auto_free_pointer(apinfo); + VALUE tmpbuf = rb_imemo_tmpbuf_auto_free_pointer(); + struct rb_ary_pattern_info *apinfo = ZALLOC(struct rb_ary_pattern_info); + rb_imemo_tmpbuf_set_ptr(tmpbuf, apinfo); node = NEW_NODE(NODE_ARYPTN, 0, 0, apinfo, loc); apinfo->imemo = tmpbuf; RB_OBJ_WRITTEN(p->ast, Qnil, tmpbuf); @@ -11699,16 +11698,19 @@ local_pop(struct parser_params *p) https://github.com/ruby/ruby/blob/trunk/parse.y#L11698 #ifndef RIPPER static ID* -local_tbl(struct parser_params *p, VALUE *tmp) +local_tbl(struct parser_params *p) { int cnt_args = vtable_size(p->lvtbl->args); int cnt_vars = vtable_size(p->lvtbl->vars); int cnt = cnt_args + cnt_vars; int i, j; ID *buf; + VALUE tbl = 0; if (cnt <= 0) return 0; + tbl = rb_imemo_tmpbuf_auto_free_pointer(); buf = ALLOC_N(ID, cnt + 2); + rb_imemo_tmpbuf_set_ptr(tbl, buf); MEMCPY(buf+1, p->lvtbl->args->tbl, ID, cnt_args); /* remove IDs duplicated to warn shadowing */ for (i = 0, j = cnt_args+1; i < cnt_vars; ++i) { @@ -11717,12 +11719,13 @@ local_tbl(struct parser_params *p, VALUE *tmp) https://github.com/ruby/ruby/blob/trunk/parse.y#L11719 buf[j++] = id; } } - if (--j < cnt) REALLOC_N(buf, ID, (cnt = j) + 2); + if (--j < cnt) { + REALLOC_N(buf, ID, (cnt = j) + 2); + rb_imemo_tmpbuf_set_ptr(tbl, buf); + } buf[0] = cnt; - - VALUE tmpbuf = rb_imemo_tmpbuf_auto_free_pointer(buf); - *tmp = tmpbuf; - buf[cnt + 1] = (ID)tmpbuf; + buf[cnt + 1] = (ID)tbl; + RB_OBJ_WRITTEN(p->ast, Qnil, tbl); return buf; } @@ -11732,11 +11735,9 @@ node_newnode_with_locals(struct parser_params *p, enum node_type type, VALUE a1, https://github.com/ruby/ruby/blob/trunk/parse.y#L11735 { ID *a0; NODE *n; - VALUE tbl = 0; - a0 = local_tbl(p, &tbl); + a0 = local_tbl(p); n = NEW_NODE(type, a0, a1, a2, loc); - if (tbl) RB_OBJ_WRITTEN(p->ast, Qnil, tbl); return n; } diff --git a/process.c b/process.c index fa5de27..00e140e 100644 --- a/process.c +++ b/process.c @@ -2691,8 +2691,8 @@ open_func(void *ptr) https://github.com/ruby/ruby/blob/trunk/process.c#L2691 static void rb_execarg_allocate_dup2_tmpbuf(struct rb_execarg *eargp, long len) { - VALUE tmpbuf = rb_imemo_tmpbuf_auto_free_pointer(NULL); - ((rb_imemo_tmpbuf_t *)tmpbuf)->ptr = ruby_xmalloc(run_exec_dup2_tmpbuf_size(len)); + VALUE tmpbuf = rb_imemo_tmpbuf_auto_free_pointer(); + rb_imemo_tmpbuf_set_ptr(tmpbuf, ruby_xmalloc(run_exec_dup2_tmpbuf_size(len))); eargp->dup2_tmpbuf = tmpbuf; } -- cgit v0.10.2 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/