Skip to content

Commit 053af73

Browse files
committed
Guard from memory leak in Psych::Emitter#start_document
When an exception is raised, it can leak memory in `head`. There are two places that can leak memory: 1. `Check_Type(tuple, T_ARRAY)` can leak memory if `tuple` is not an array. 2. `StringValue(name)` and `StringValue(value)` if they are not strings and the call to `to_str` does not return a string. This commit fixes these memory leaks by wrapping the code around a rb_ensure so that the memory is freed in all cases. The following code demonstrates the memory leak: emitter = Psych::Emitter.new(StringIO.new) nil_to_string_tags = [[nil, "tag:TALOS"]] + ([1] * 1000) expected_array_tags = [1] * 1000 10.times do 1_000.times do # Raises `no implicit conversion of nil into String` emitter.start_document([], nil_to_string_tags, 0) rescue TypeError end 1_000.times do # Raises `wrong argument type Integer (expected Array)` emitter.start_document([], expected_array_tags, 0) rescue TypeError end puts `ps -o rss= -p #{$$}` end Before: 47248 79728 111968 144224 176480 208896 241104 273280 305472 337664 After: 14832 15088 15344 15344 15360 15632 15632 15632 15648 15648
1 parent 1242cfe commit 053af73

File tree

1 file changed

+50
-16
lines changed

1 file changed

+50
-16
lines changed

ext/psych/psych_emitter.c

Lines changed: 50 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -136,23 +136,29 @@ static VALUE end_stream(VALUE self)
136136
return self;
137137
}
138138

139-
/* call-seq: emitter.start_document(version, tags, implicit)
140-
*
141-
* Start a document emission with YAML +version+, +tags+, and an +implicit+
142-
* start.
143-
*
144-
* See Psych::Handler#start_document
145-
*/
146-
static VALUE start_document(VALUE self, VALUE version, VALUE tags, VALUE imp)
139+
struct start_document_data {
140+
VALUE self;
141+
VALUE version;
142+
VALUE tags;
143+
VALUE imp;
144+
145+
yaml_tag_directive_t * head;
146+
};
147+
148+
static VALUE start_document_try(VALUE d)
147149
{
150+
struct start_document_data * data = (struct start_document_data *)d;
151+
VALUE self = data->self;
152+
VALUE version = data->version;
153+
VALUE tags = data->tags;
154+
VALUE imp = data->imp;
155+
148156
yaml_emitter_t * emitter;
149-
yaml_tag_directive_t * head = NULL;
150157
yaml_tag_directive_t * tail = NULL;
151158
yaml_event_t event;
152159
yaml_version_directive_t version_directive;
153160
TypedData_Get_Struct(self, yaml_emitter_t, &psych_emitter_type, emitter);
154161

155-
156162
Check_Type(version, T_ARRAY);
157163

158164
if(RARRAY_LEN(version) > 0) {
@@ -171,8 +177,8 @@ static VALUE start_document(VALUE self, VALUE version, VALUE tags, VALUE imp)
171177
Check_Type(tags, T_ARRAY);
172178

173179
len = RARRAY_LEN(tags);
174-
head = xcalloc((size_t)len, sizeof(yaml_tag_directive_t));
175-
tail = head;
180+
data->head = xcalloc((size_t)len, sizeof(yaml_tag_directive_t));
181+
tail = data->head;
176182

177183
for(i = 0; i < len && i < RARRAY_LEN(tags); i++) {
178184
VALUE tuple = RARRAY_AREF(tags, i);
@@ -182,9 +188,9 @@ static VALUE start_document(VALUE self, VALUE version, VALUE tags, VALUE imp)
182188
Check_Type(tuple, T_ARRAY);
183189

184190
if(RARRAY_LEN(tuple) < 2) {
185-
xfree(head);
186191
rb_raise(rb_eRuntimeError, "tag tuple must be of length 2");
187192
}
193+
188194
name = RARRAY_AREF(tuple, 0);
189195
value = RARRAY_AREF(tuple, 1);
190196
StringValue(name);
@@ -202,18 +208,46 @@ static VALUE start_document(VALUE self, VALUE version, VALUE tags, VALUE imp)
202208
yaml_document_start_event_initialize(
203209
&event,
204210
(RARRAY_LEN(version) > 0) ? &version_directive : NULL,
205-
head,
211+
data->head,
206212
tail,
207213
imp ? 1 : 0
208214
);
209215

210216
emit(emitter, &event);
211217

212-
if(head) xfree(head);
213-
214218
return self;
215219
}
216220

221+
static VALUE start_document_ensure(VALUE d)
222+
{
223+
struct start_document_data * data = (struct start_document_data *)d;
224+
225+
xfree(data->head);
226+
227+
return Qnil;
228+
}
229+
230+
/* call-seq: emitter.start_document(version, tags, implicit)
231+
*
232+
* Start a document emission with YAML +version+, +tags+, and an +implicit+
233+
* start.
234+
*
235+
* See Psych::Handler#start_document
236+
*/
237+
static VALUE start_document(VALUE self, VALUE version, VALUE tags, VALUE imp)
238+
{
239+
struct start_document_data data = {
240+
.self = self,
241+
.version = version,
242+
.tags = tags,
243+
.imp = imp,
244+
245+
.head = NULL,
246+
};
247+
248+
return rb_ensure(start_document_try, (VALUE)&data, start_document_ensure, (VALUE)&data);
249+
}
250+
217251
/* call-seq: emitter.end_document(implicit)
218252
*
219253
* End a document emission with an +implicit+ ending.

0 commit comments

Comments
 (0)