[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [xen master] tools/ocaml/evtchn: Don't reference Custom objects with the GC lock released
commit 2636d8ff7a670c4d2485757dbe966e36c259a960 Author: Edwin Török <edwin.torok@xxxxxxxxx> AuthorDate: Thu Jan 12 17:48:29 2023 +0000 Commit: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> CommitDate: Mon Feb 6 10:22:12 2023 +0000 tools/ocaml/evtchn: Don't reference Custom objects with the GC lock released The modification to the _H() macro for Ocaml 5 support introduced a subtle bug. 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." Previously, the value was a naked C pointer, so dereferencing it wasn't "accessing any Ocaml data", but the fix to avoid naked C pointers added a layer of indirection through an Ocaml Custom object, meaning that the common pattern of using _H() in a blocking section is unsafe. In order to fix: * Drop the _H() macro and replace it with a static inline xce_of_val(). * Opencode the assignment into Data_custom_val() in the two constructors. * Rename "value xce" parameters to "value xce_val" so we can consistently have "xenevtchn_handle *xce" on the stack, and obtain the pointer with the GC lock still held. Fixes: 22d5affdf0ce ("tools/ocaml/evtchn: OCaml 5 support, fix potential resource leak") Signed-off-by: Edwin Török <edwin.torok@xxxxxxxxx> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> Acked-by: Christian Lindig <christian.lindig@xxxxxxxxxx> --- tools/ocaml/libs/eventchn/xeneventchn_stubs.c | 60 ++++++++++++++++----------- 1 file changed, 35 insertions(+), 25 deletions(-) diff --git a/tools/ocaml/libs/eventchn/xeneventchn_stubs.c b/tools/ocaml/libs/eventchn/xeneventchn_stubs.c index aa8a69cc1e..d7881ca95f 100644 --- a/tools/ocaml/libs/eventchn/xeneventchn_stubs.c +++ b/tools/ocaml/libs/eventchn/xeneventchn_stubs.c @@ -33,11 +33,14 @@ #include <caml/fail.h> #include <caml/signals.h> -#define _H(__h) (*((xenevtchn_handle **)Data_custom_val(__h))) +static inline xenevtchn_handle *xce_of_val(value v) +{ + return *(xenevtchn_handle **)Data_custom_val(v); +} static void stub_evtchn_finalize(value v) { - xenevtchn_close(_H(v)); + xenevtchn_close(xce_of_val(v)); } static struct custom_operations xenevtchn_ops = { @@ -68,7 +71,7 @@ CAMLprim value stub_eventchn_init(value cloexec) caml_failwith("open failed"); result = caml_alloc_custom(&xenevtchn_ops, sizeof(xce), 0, 1); - _H(result) = xce; + *(xenevtchn_handle **)Data_custom_val(result) = xce; CAMLreturn(result); } @@ -87,18 +90,19 @@ CAMLprim value stub_eventchn_fdopen(value fdval) caml_failwith("evtchn fdopen failed"); result = caml_alloc_custom(&xenevtchn_ops, sizeof(xce), 0, 1); - _H(result) = xce; + *(xenevtchn_handle **)Data_custom_val(result) = xce; CAMLreturn(result); } -CAMLprim value stub_eventchn_fd(value xce) +CAMLprim value stub_eventchn_fd(value xce_val) { - CAMLparam1(xce); + CAMLparam1(xce_val); CAMLlocal1(result); + xenevtchn_handle *xce = xce_of_val(xce_val); int fd; - fd = xenevtchn_fd(_H(xce)); + fd = xenevtchn_fd(xce); if (fd == -1) caml_failwith("evtchn fd failed"); @@ -107,13 +111,14 @@ CAMLprim value stub_eventchn_fd(value xce) CAMLreturn(result); } -CAMLprim value stub_eventchn_notify(value xce, value port) +CAMLprim value stub_eventchn_notify(value xce_val, value port) { - CAMLparam2(xce, port); + CAMLparam2(xce_val, port); + xenevtchn_handle *xce = xce_of_val(xce_val); int rc; caml_enter_blocking_section(); - rc = xenevtchn_notify(_H(xce), Int_val(port)); + rc = xenevtchn_notify(xce, Int_val(port)); caml_leave_blocking_section(); if (rc == -1) @@ -122,15 +127,16 @@ CAMLprim value stub_eventchn_notify(value xce, value port) CAMLreturn(Val_unit); } -CAMLprim value stub_eventchn_bind_interdomain(value xce, value domid, +CAMLprim value stub_eventchn_bind_interdomain(value xce_val, value domid, value remote_port) { - CAMLparam3(xce, domid, remote_port); + CAMLparam3(xce_val, domid, remote_port); CAMLlocal1(port); + xenevtchn_handle *xce = xce_of_val(xce_val); xenevtchn_port_or_error_t rc; caml_enter_blocking_section(); - rc = xenevtchn_bind_interdomain(_H(xce), Int_val(domid), Int_val(remote_port)); + rc = xenevtchn_bind_interdomain(xce, Int_val(domid), Int_val(remote_port)); caml_leave_blocking_section(); if (rc == -1) @@ -140,14 +146,15 @@ CAMLprim value stub_eventchn_bind_interdomain(value xce, value domid, CAMLreturn(port); } -CAMLprim value stub_eventchn_bind_virq(value xce, value virq_type) +CAMLprim value stub_eventchn_bind_virq(value xce_val, value virq_type) { - CAMLparam2(xce, virq_type); + CAMLparam2(xce_val, virq_type); CAMLlocal1(port); + xenevtchn_handle *xce = xce_of_val(xce_val); xenevtchn_port_or_error_t rc; caml_enter_blocking_section(); - rc = xenevtchn_bind_virq(_H(xce), Int_val(virq_type)); + rc = xenevtchn_bind_virq(xce, Int_val(virq_type)); caml_leave_blocking_section(); if (rc == -1) @@ -157,13 +164,14 @@ CAMLprim value stub_eventchn_bind_virq(value xce, value virq_type) CAMLreturn(port); } -CAMLprim value stub_eventchn_unbind(value xce, value port) +CAMLprim value stub_eventchn_unbind(value xce_val, value port) { - CAMLparam2(xce, port); + CAMLparam2(xce_val, port); + xenevtchn_handle *xce = xce_of_val(xce_val); int rc; caml_enter_blocking_section(); - rc = xenevtchn_unbind(_H(xce), Int_val(port)); + rc = xenevtchn_unbind(xce, Int_val(port)); caml_leave_blocking_section(); if (rc == -1) @@ -172,14 +180,15 @@ CAMLprim value stub_eventchn_unbind(value xce, value port) CAMLreturn(Val_unit); } -CAMLprim value stub_eventchn_pending(value xce) +CAMLprim value stub_eventchn_pending(value xce_val) { - CAMLparam1(xce); + CAMLparam1(xce_val); CAMLlocal1(result); + xenevtchn_handle *xce = xce_of_val(xce_val); xenevtchn_port_or_error_t port; caml_enter_blocking_section(); - port = xenevtchn_pending(_H(xce)); + port = xenevtchn_pending(xce); caml_leave_blocking_section(); if (port == -1) @@ -189,16 +198,17 @@ CAMLprim value stub_eventchn_pending(value xce) CAMLreturn(result); } -CAMLprim value stub_eventchn_unmask(value xce, value _port) +CAMLprim value stub_eventchn_unmask(value xce_val, value _port) { - CAMLparam2(xce, _port); + CAMLparam2(xce_val, _port); + xenevtchn_handle *xce = xce_of_val(xce_val); evtchn_port_t port; int rc; port = Int_val(_port); caml_enter_blocking_section(); - rc = xenevtchn_unmask(_H(xce), port); + rc = xenevtchn_unmask(xce, port); caml_leave_blocking_section(); if (rc) -- generated by git-patchbot for /home/xen/git/xen.git#master
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |