Port driver use-after-free

--------------EFFF1011FBAC280F90E155C4
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Transfer-Encoding: 7bit

Hello,

Failure scenario:
1. A process calls erlang:port_control() and passes a binary to port driver.
2. The driver is not invoked immediately, so the binary's refc is 
incremented and the pointers to binary and to binary's data are kept in 
struct ErtsProc2PortSigData_ for later call. The process is in pending 
queue.
3. The pending process is the only one having the refcount to binary.
4. An event occurs causing garbage collecting of the pending process.
5. The binary is relocated, so the pointers kept in 
ErtsProc2PortSigData_ become invalid.
6. The driver manipulates with already freed data.

Unfortunately, I don't have the minimal code sample reproducing the issue.
The described behavior is observed only on high loads and leads to VM crash.
The issue presents in  OTP-18.3 release. I didn't try it for later 
releases, but I couldn't find any related fixes done either.

The memory for binary was first allocated as:
0x481a39 <do_erts_alcu_alloc+270>
0x481c06 <erts_alcu_alloc_thr_pref+135>
0x58db08 <erts_alloc+75>
0x58dc91 <erts_bin_nrml_alloc+68>
0x591362 <erts_bs_append+1566>
0x44177b <process_main+51114>
0x508a27 <sched_thread_func+499>
0x68f72d <thr_wrapper+235>

And then reallocated as:
0x482026 <do_erts_alcu_realloc+190>
0x4828f9 <realloc_thr_pref+257>
0x482ac1 <erts_alcu_realloc_thr_pref+51>
0x585984 <erts_realloc_fnf+81>
0x586200 <erts_bin_realloc+110>
0x58caac <sweep_off_heap+1277>
0x58a37e <major_collection+3163>
0x586ca8 <erts_garbage_collect+493>
0x43c36b <process_main+29594>
0x508a27 <sched_thread_func+499>
0x68f72d <thr_wrapper+235>

In the attachment there is a patch with a quick fix for the issue.
The idea is to always copy the data passed to the port driver if the 
actual call is pended.
It is fine for small data, but can lead to performance degradation if 
megabytes-size binaries are passed to port_control(), that's why I 
haven't done a PR.

Salikhov Dinislam

--------------EFFF1011FBAC280F90E155C4
Content-Type: text/x-patch; name="erlang.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment; filename="erlang.patch"

diff --git a/erts/emulator/beam/io.c b/erts/emulator/beam/io.c
index 81a05a6b8..09c88b5af 100644
--- a/erts/emulator/beam/io.c
+++ b/erts/emulator/beam/io.c
@@ -3969,7 +3969,7 @@ erts_port_control(Process* c_p,
     int tmp_alloced = 0;
     erts_aint32_t sched_flags;
     Binary *binp;
-    int copy;
+    int copy = 1;
     ErtsProc2PortSigData *sigdp;
 
     sched_flags = erts_smp_atomic32_read_nob(&prt->sched.flags);
@@ -3993,6 +3993,7 @@ erts_port_control(Process* c_p,
 		return ERTS_PORT_OP_BADARG;
 	    bufp = erts_alloc(ERTS_ALC_T_DRV_CTRL_DATA, size);
 	    r = erts_iolist_to_buf(data, bufp, size);
+	    copy = 0;
 	    ASSERT(r == 0);
 	}
 	else {
@@ -4087,26 +4088,8 @@ erts_port_control(Process* c_p,
 
     /* Convert data into something that can be scheduled */
 
-    copy = tmp_alloced;
-
     binp = NULL;
 
-    if (is_binary(data) && binary_bitoffset(data) == 0) {
-	Eterm *ebinp = binary_val_rel(data, NULL);
-	ASSERT(!tmp_alloced);
-	if (*ebinp == HEADER_SUB_BIN)
-	    ebinp = binary_val_rel(((ErlSubBin *) ebinp)->orig, NULL);
-	if (*ebinp != HEADER_PROC_BIN)
-	    copy = 1;
-	else {
-	    binp = ((ProcBin *) ebinp)->val;
-	    ASSERT(bufp <= bufp + size);
-	    ASSERT(binp->orig_bytes <= bufp
-		   && bufp + size <= binp->orig_bytes + binp->orig_size);
-	    erts_refc_inc(&binp->refc, 1);
-	}
-    }
-
     if (copy) {
 	char *old_bufp = bufp;
 	bufp = erts_alloc(ERTS_ALC_T_DRV_CTRL_DATA, size);

--------------EFFF1011FBAC280F90E155C4
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

_______________________________________________
erlang-bugs mailing list
[email protected]
http://erlang.org/mailman/listinfo/erlang-bugs

--------------EFFF1011FBAC280F90E155C4--