1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
|
From 1fdff77e26290ae1ed40e8253959d12a0c4b3d3f Mon Sep 17 00:00:00 2001
From: Andrew Cooper <andrew.cooper3@citrix.com>
Date: Tue, 31 Jan 2023 17:19:30 +0000
Subject: [PATCH 20/61] tools/ocaml/xc: Don't reference Abstract_Tag objects
with the GC lock released
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>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
(cherry picked from commit 9e7c74e6f9fd2e44df1212643b80af9032b45b07)
---
tools/ocaml/libs/xc/xenctrl_stubs.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c b/tools/ocaml/libs/xc/xenctrl_stubs.c
index e2efcbe182..0a0fe45c54 100644
--- a/tools/ocaml/libs/xc/xenctrl_stubs.c
+++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
@@ -937,26 +937,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);
}
--
2.40.0
|