]> xenbits.xensource.com Git - xcp/xen-api-libs.git/commitdiff
CA-38105: Fix a heap corruption in the stub_unix_recv_fd C fn.
authorDavid Scott <dave.scott@eu.citrix.com>
Mon, 8 Mar 2010 11:28:56 +0000 (11:28 +0000)
committerDavid Scott <dave.scott@eu.citrix.com>
Mon, 8 Mar 2010 11:28:56 +0000 (11:28 +0000)
The ocaml manual states
"caml_alloc_small(n, t) returns a fresh small block of size n <= Max_young_wosize words, with tag t. If this block is a structured block (i.e. if t < No_scan_tag), then the fields of the block (initially containing garbage) must be initialized with legal values (using direct assignment to the fields of the block) before the next allocation."

This function returns a tuple including an AF_UNIX value of type Unix.sockaddr where:
  type sockaddr =
  | AF_UNIX of string
  | ...

Unfortunately the C fn called copy_string() to add the string to the heap block *without setting the field of the block*. This means that the field contains garbage so, in the rare event of the string allocation exhausting the minor heap and invoking a GC, the GC follows the garbage pointer and crashes. Helpfully the stack makes this obvious:

Program terminated with signal 11, Segmentation fault.
[New process 19425]
#0  0x0808135a in caml_oldify_mopup ()
(gdb) bt
#0  0x0808135a in caml_oldify_mopup ()
#1  0x08081416 in caml_empty_minor_heap ()
#2  0x080814fc in caml_minor_collection ()
#3  0x080820cc in caml_alloc_string ()
#4  0x0808216c in caml_copy_string ()
#5  0x080786d2 in stub_unix_recv_fd ()
#6  0x0805536f in camlFecomms__receive_named_fd_91 ()

The fix is to initialise the field to a legal value before calling copy_string. Note that the field is unconditionally reset to a proper string type in the following if statement.

Signed-off-by: David Scott <dave.scott@eu.citrix.com>
stdext/unixext_stubs.c

index 7941b5a1c98f22b62d89f809601b46dca74603e9..a681605ebfd0c6fcbe58a22cf9a77d6e20c7d703 100644 (file)
@@ -398,8 +398,9 @@ CAMLprim value stub_unix_recv_fd(value sock, value buff, value ofs, value len, v
 
   memmove(&Byte(buff, Long_val(ofs)), iobuf, numbytes);
 
-  addr=alloc_small(1,0);
-  
+  addr=alloc_small(1,0); /* Unix.sockaddr; must be an ADDR_UNIX of string */
+  Field(addr, 0) = Val_unit; /* must set all fields before next allocation */
+
   if(ret>0) {
     Field(addr,0) = copy_string(unix_socket_name.sun_path);
   } else {