From b3e5dc8edf45dca5ffcabe5a89380b3a4947a955 Mon Sep 17 00:00:00 2001 From: Sverker Eriksson Date: Thu, 6 Oct 2011 11:11:00 +0200 Subject: erts: Fix write-after-free bug in inet driver Bug caught by valgrind but never confirmed as cause of a real problem. Triggered by emulator_test/distribution_SUITE:bad_dist_structure when bad data is received to a distribution port. Port is closed by erts_net_message() and the freed port data is then later used when returning to inet_drv. Everything happens in the same call to tcp_inet_input(). ==32592== Invalid read of size 8 ==32592== at 0x5B6D8C: tcp_deliver (inet_drv.c:8536) ==32592== by 0x5B720E: tcp_recv (inet_drv.c:8652) ==32592== by 0x5B7753: tcp_inet_input (inet_drv.c:8990) ==32592== by 0x5B7FA5: tcp_inet_drv_input (inet_drv.c:9257) ==32592== by 0x4B900C: erts_port_task_execute (erl_port_task.c:856) ==32592== by 0x4AC873: schedule (erl_process.c:5511) ==32592== by 0x5527CA: process_main (beam_emu.c:1225) ==32592== by 0x4A9958: sched_thread_func (erl_process.c:3789) ==32592== by 0x5E2971: thr_wrapper (ethread.c:106) ==32592== by 0x4FEA142: start_thread (in /lib64/libpthread-2.4.so) ==32592== by 0x52C374C: clone (in /lib64/libc-2.4.so) ==32592== Address 0x9b13ea8 is 440 bytes inside a block of size 488 free'd ==32592== at 0x4A1FE94: free (vg_replace_malloc.c:366) ==32592== by 0x58AE69: erts_sys_free (sys.c:2588) ==32592== by 0x48E54F: erts_free (erl_alloc.h:218) ==32592== by 0x497390: driver_free (io.c:3527) ==32592== by 0x5B2E77: inet_stop (inet_drv.c:7152) ==32592== by 0x5B502F: tcp_inet_stop (inet_drv.c:7855) ==32592== by 0x492E60: terminate_port (io.c:1849) ==32592== by 0x493971: erts_do_exit_port (io.c:2059) ==32592== by 0x4CEE01: erts_net_message (dist.c:1508) ==32592== by 0x496AC1: driver_output2 (io.c:3294) ==32592== by 0x5AAA69: inet_port_data (inet_drv.c:1894) ==32592== by 0x5AD57E: tcp_reply_data (inet_drv.c:3181) ==32592== by 0x5B6D84: tcp_deliver (inet_drv.c:8533) ==32592== by 0x5B720E: tcp_recv (inet_drv.c:8652) ==32592== by 0x5B7753: tcp_inet_input (inet_drv.c:8990) Fixed by checking the negative return code earlier in tcp_deliver(). --- erts/emulator/drivers/common/inet_drv.c | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) (limited to 'erts') diff --git a/erts/emulator/drivers/common/inet_drv.c b/erts/emulator/drivers/common/inet_drv.c index 43114c6039..5c4fd7bab3 100644 --- a/erts/emulator/drivers/common/inet_drv.c +++ b/erts/emulator/drivers/common/inet_drv.c @@ -8497,32 +8497,29 @@ static int tcp_deliver(tcp_descriptor* desc, int len) } while (len > 0) { - int code = 0; + int code; inet_input_count(INETP(desc), len); /* deliver binary? */ if (len*4 >= desc->i_buf->orig_size*3) { /* >=75% */ + code = tcp_reply_binary_data(desc, desc->i_buf, + (desc->i_ptr_start - + desc->i_buf->orig_bytes), + len); + if (code < 0) + return code; + /* something after? */ if (desc->i_ptr_start + len == desc->i_ptr) { /* no */ - code = tcp_reply_binary_data(desc, desc->i_buf, - (desc->i_ptr_start - - desc->i_buf->orig_bytes), - len); tcp_clear_input(desc); } else { /* move trail to beginning of a new buffer */ - ErlDrvBinary* bin; + ErlDrvBinary* bin = alloc_buffer(desc->i_bufsz); char* ptr_end = desc->i_ptr_start + len; int sz = desc->i_ptr - ptr_end; - bin = alloc_buffer(desc->i_bufsz); memcpy(bin->orig_bytes, ptr_end, sz); - - code = tcp_reply_binary_data(desc, desc->i_buf, - (desc->i_ptr_start- - desc->i_buf->orig_bytes), - len); free_buffer(desc->i_buf); desc->i_buf = bin; desc->i_ptr_start = desc->i_buf->orig_bytes; @@ -8534,17 +8531,15 @@ static int tcp_deliver(tcp_descriptor* desc, int len) code = tcp_reply_data(desc, desc->i_ptr_start, len); /* XXX The buffer gets thrown away on error (code < 0) */ /* Windows needs workaround for this in tcp_inet_event... */ + if (code < 0) + return code; desc->i_ptr_start += len; if (desc->i_ptr_start == desc->i_ptr) tcp_clear_input(desc); else desc->i_remain = 0; - } - if (code < 0) - return code; - count++; len = 0; -- cgit v1.2.3