@@ -1028,26 +1028,25 @@ CAMLprim value stub_map_foreign_range(value xch, value dom,
CAMLparam4(xch, dom, size, mfn);
CAMLlocal1(result);
struct mmap_interface *intf;
- uint32_t c_dom;
- unsigned long c_mfn;
+ unsigned long c_mfn = Nativeint_val(mfn);
+ int len = Int_val(size);
+ void *ptr;
BUILD_BUG_ON((sizeof(struct mmap_interface) % sizeof(value)) != 0);
result = caml_alloc(Wsize_bsize(sizeof(struct mmap_interface)),
Abstract_tag);
- intf = (struct mmap_interface *) result;
-
- intf->len = Int_val(size);
-
- c_dom = _D(dom);
- c_mfn = Nativeint_val(mfn);
caml_enter_blocking_section();
- intf->addr = xc_map_foreign_range(_H(xch), c_dom,
- intf->len, PROT_READ|PROT_WRITE,
- c_mfn);
+ ptr = xc_map_foreign_range(_H(xch), _D(dom), len,
+ PROT_READ|PROT_WRITE, c_mfn);
caml_leave_blocking_section();
- if (!intf->addr)
+
+ if (!ptr)
caml_failwith("xc_map_foreign_range error");
+
+ intf = Data_abstract_val(result);
+ *intf = (struct mmap_interface){ ptr, len };
+
CAMLreturn(result);
}
The intf->{addr,len} references in the xc_map_foreign_range() call are unsafe. From the manual: https://ocaml.org/manual/intfc.html#ss:parallel-execution-long-running-c-code "After caml_release_runtime_system() was called and until caml_acquire_runtime_system() is called, the C code must not access any OCaml data, nor call any function of the run-time system, nor call back into OCaml code." More than what the manual says, the intf pointer is (potentially) invalidated by caml_enter_blocking_section() if another thread happens to perform garbage collection at just the right (wrong) moment. Rewrite the logic. There's no need to stash data in the Ocaml object until the success path at the very end. Fixes: 8b7ce06a2d34 ("ocaml: Add XC bindings.") Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Christian Lindig <christian.lindig@citrix.com> CC: David Scott <dave@recoil.org> CC: Edwin Török <edwin.torok@cloud.com> CC: Rob Hoes <Rob.Hoes@citrix.com> Note: the mmap stub has a similar pattern when constructing a mmap_interface, but, but it's not actually unsafe because it doesn't drop the GC lock. _H() is buggy too, but this patch needs backporting further than that fix. --- tools/ocaml/libs/xc/xenctrl_stubs.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-)