[Xen-API] [PATCH] Fix heap corruption/segfault in C stub

[David Scott <dave.scott@xxxxxxxxxxxxx>]

# HG changeset patch
# User David Scott <dave.scott@xxxxxxxxxxxxx>
# Date 1267812178 0
# Node ID 125e9e69ecf59f3bbc022dde657e02d7221a70c1
# Parent  262d2e935f2953c1fdca378c534c4fb5e20a8c7a
CA-38105: Fix a heap corruption in the stub_unix_recv_fd C fn.

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@xxxxxxxxxxxxx>

diff -r 262d2e935f29 -r 125e9e69ecf5 stdext/unixext_stubs.c
--- a/stdext/unixext_stubs.c    Wed Feb 17 09:05:03 2010 +0000
+++ b/stdext/unixext_stubs.c    Fri Mar 05 18:02:58 2010 +0000
@@ -398,8 +398,9 @@
 
   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 {
1 file changed, 3 insertions(+), 2 deletions(-)
stdext/unixext_stubs.c |    5 +++--

Attachment: xen-api-libs.hg.patch
Description: Text Data

_______________________________________________
xen-api mailing list
xen-api@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/mailman/listinfo/xen-api