Repository navigation
Heap corruption when an exception is triggered during demarshaling #14007
Description
A segmentation fault may occur when a GC is triggered due to the allocation of an exception during demarshaling.
Step to reproduce
1/ Write the two following programs
(* Source file p1.ml *)
let l =
let acc = ref [] in
for i = 1 to 1000 do
acc := i :: !acc
done;
!acc in
let f x = x + 1 in
let pair = (l, f) in
let str = Marshal.to_string pair [ Marshal.Closures ] in
let oc = open_out_bin "data.bin" in
output_string oc str;
close_out oc;
(* Source file p2.ml *)
let str =
let ic = open_in_bin "data.bin" in
let len = in_channel_length ic in
let str = really_input_string ic len in
close_in ic;
str in
for i = 1 to 1_000_000_000 do
try ignore (Marshal.from_string str 0);
with _ -> ()
done
2/ Compile with:
ocamlopt p1.ml -o p1
ocamlopt p2.ml -o p2
3/ Generate file data.bin thanks to p1:
./p1
4/ Run p2
./p2
Erreur de segmentation (core dumped)
Explanation
In short: a root registered in C code using CAMLlocal1 [intern.c:938] points to partially initialized blocks when the GC is triggered for the allocation of the exception when calling caml_failwith [intern.c:1073].
Long version: when the minor heap is almost full:
- Marshal.from_string calls caml_input_val_from_bytes [runtime/intern.c:935]
- caml_input_val_from_bytes registers a heap root thanks to CAMLlocal1 (obj); [runtime/intern.c:938]
- the address of obj is passed to intern_rec [runtime/intern.c:953]
- intern_rec starts to build obj with a tree of blocks, the last one being allocated but not yet initialized
- intern_rec detects a demarshaling problem (here it is due to an unknown code module, but it also works with other problems like an incompatible OCaml version or an incompatible architecture)
- intern_rec calls intern_bad_code_pointer [runtime/intern.c:646]
- intern_bad_code_pointer calls caml_failwith [runtime/intern.c:1073]
- caml_failwith tries to allocate the exception
- the allocation triggers a garbage collection
- the garbage collector follows the partially initialized memory tree registered at step 2 thanks to CAMLlocal1(obj)
- => Segmentation Fault!
The stack trace is:
#0 do_some_marking (stk=0x5555555ef490, budget=16261, budget@entry=16384) at runtime/major_gc.c:1001
#1 0x00005555555a07b5 in mark (budget=budget@entry=16384) at runtime/major_gc.c:1131
#2 0x00005555555a0ce7 in major_collection_slice (howmuch=<optimized out>, participant_count=participant_count@entry=0, barrier_participants=barrier_participants@entry=0x0,
mode=mode@entry=Slice_interruptible, force_compaction=0) at runtime/major_gc.c:1711
#3 0x00005555555a1736 in caml_major_collection_slice (howmuch=howmuch@entry=-1) at runtime/major_gc.c:1891
#4 0x0000555555590179 in caml_poll_gc_work () at runtime/domain.c:1874
#5 0x00005555555a6935 in caml_alloc_small_dispatch (dom_st=dom_st@entry=0x5555555ee480, wosize=wosize@entry=9, flags=flags@entry=1, nallocs=nallocs@entry=1,
encoded_alloc_lens=encoded_alloc_lens@entry=0x0) at runtime/minor_gc.c:906
#6 0x000055555558b5a8 in caml_alloc_string (len=len@entry=65) at runtime/alloc.c:183
#7 0x000055555558b5dd in caml_alloc_initialized_string (p=0x7fffffffdac0 "input_value: unknown code module 245256F65552FDD5EAC754CE8128BC34", len=65) at runtime/alloc.c:197
#8 caml_copy_string (s=s@entry=0x7fffffffdac0 "input_value: unknown code module 245256F65552FDD5EAC754CE8128BC34") at runtime/alloc.c:218
#9 0x000055555559419f in caml_exception_with_string (exn_constr=<optimized out>, msg=msg@entry=0x7fffffffdac0 "input_value: unknown code module 245256F65552FDD5EAC754CE8128BC34") at runtime/fail.c:65
#10 0x00005555555b1a03 in caml_exception_failure (msg=msg@entry=0x7fffffffdac0 "input_value: unknown code module 245256F65552FDD5EAC754CE8128BC34") at runtime/fail_nat.c:94
#11 0x000055555559424f in caml_failwith (msg=msg@entry=0x7fffffffdac0 "input_value: unknown code module 245256F65552FDD5EAC754CE8128BC34") at runtime/fail.c:95
#12 0x00005555555983ac in intern_bad_code_pointer (digest=digest@entry=0x7fffffffdc00 "$RV\366UR\375\325\352\307T\316\201(\2744\260\315cUUU") at runtime/intern.c:1073
#13 0x00005555555993a5 in intern_rec (s=s@entry=0x55555563cdb0, fun_name=fun_name@entry=0x5555555bb63b "input_val_from_string", dest=0x7fffae6e7608, dest@entry=0x7fffffffdc80) at runtime/intern.c:646
#14 0x0000555555599969 in caml_input_val_from_bytes (str=<optimized out>, ofs=<optimized out>) at runtime/intern.c:953
Solution
Simply do not register obj as a heap root, like in caml_input_val [runtime/intern.c:859].
Here is a patch:
--- old/runtime/intern.c 2025-01-08 10:01:04.000000000 +0100
+++ new/runtime/intern.c 2025-05-02 03:51:35.247154026 +0200
@@ -935,7 +935,7 @@
CAMLexport value caml_input_val_from_bytes(value str, intnat ofs)
{
CAMLparam1 (str);
- CAMLlocal1 (obj);
+ value obj;
struct marshal_header h;
struct caml_intern_state* s = init_intern_state ();
Activity
gasche commentedon May 2, 2025
Nice catch and impressive bug-tracking!
My impression is that the fix you propose is not the end of the story.
The code with the patch is weird, because we use the rooting mechanism for some of the OCaml values involved but not all of them. I wish we could write it in a way to not use CAMLparam/CAMLreturn at all to make it more consistent. If I understand things correctly, we could in fact just drop them all, because the only allocations occur in the exceptional case and at this point we don't care if
strgets invalidated by the GC.There is something odd with the failure logic in this file. Some parts of it are careful to call dedicated helpers
intern_failwith2(after cleanup) orinternal_cleanup_failwith3, and some parts callcaml_failwithdirectly.I think that the invariant we would want for robustness is that, after calling
intern_cleanup, then we are back to the usual OCaml world where all runtime operations are permissible. I view this as the root source of the bug at play here, we callintern_cleanup(and then allow ourselves to allocate in the exceptional path) but there is an invalid value on the heap. Ifobjis the only part that violates this invariant, then it could be pre-initialized to a valid value or it could be stored in the intern statesto be filled right before on cleanup.bvaugon commentedon May 5, 2025
Hello Gabriel,
Thank you for the answer. About your remarks:
The CAMLparam1(str) [intern.c:937] cannot be simply removed since a GC may occur when calling intern_alloc_storage [intern.c:948] and move str. However, as you suggest, between the first call to intern_rec and the call to intern_cleanup, no garbage collection may be triggered and no root is needed.
Yes, there are multiple ways to fail in this algorithm.
Pre-initialize obj is not enough to fix this bug, CAMLlocal1 already initialize obj, but yes, another way to fix this bug is to register the address of obj in the caml_intern_state structure and to reset it (for example to Val_unit) in the intern_cleanup function.
gasche commentedon May 5, 2025
As a first step, could you submit your patch as a PR, including a code comment that explains why
objshould not be registered as a root?nojb commentedon May 14, 2025
Fixed in #14015