tools/ocaml/libs: Fix memory/resource leaks with caml_alloc_custom()

Commit Message

Andrew Cooper Feb. 1, 2023, 1:29 p.m. UTC

All caml_alloc_*() functions can throw exceptions, and longjump out of
context.  If this happens, we leak the xch/xce handle.

Reorder the logic to allocate the the Ocaml object first.

Fixes: 8b3c06a3e545 ("tools/ocaml/xenctrl: OCaml 5 support, fix use-after-free")
Fixes: 22d5affdf0ce ("tools/ocaml/evtchn: OCaml 5 support, fix potential resource leak")
Signed-off-by: Andrew Cooper <[e-mail]>
---
CC: Christian Lindig <[e-mail]>
CC: David Scott <[e-mail]>
CC: Edwin Török <[e-mail]>
CC: Rob Hoes <[e-mail]>
---
 tools/ocaml/libs/eventchn/xeneventchn_stubs.c | 6 ++++--
 tools/ocaml/libs/xc/xenctrl_stubs.c           | 3 ++-
 2 files changed, 6 insertions(+), 3 deletions(-)

Comments

Christian Lindig Feb. 1, 2023, 3:34 p.m. UTC | #1

> On 1 Feb 2023, at 13:29, Andrew Cooper <[e-mail]> wrote:
> 
> All caml_alloc_*() functions can throw exceptions, and longjump out of
> context.  If this happens, we leak the xch/xce handle.
> 
> Reorder the logic to allocate the the Ocaml object first.
> 
> Fixes: 8b3c06a3e545 ("tools/ocaml/xenctrl: OCaml 5 support, fix use-after-free")
> Fixes: 22d5affdf0ce ("tools/ocaml/evtchn: OCaml 5 support, fix potential resource leak")
> Signed-off-by: Andrew Cooper <[e-mail]>
> ---
> CC: Christian Lindig <[e-mail]>
> CC: David Scott <[e-mail]>
> CC: Edwin Török <[e-mail]>
> CC: Rob Hoes <[e-mail]>
> ---
> tools/ocaml/libs/eventchn/xeneventchn_stubs.c | 6 ++++--
> tools/ocaml/libs/xc/xenctrl_stubs.c           | 3 ++-
> 2 files changed, 6 insertions(+), 3 deletions(-)

Acked-by: Christian Lindig <[e-mail]>

Patch

diff --git a/tools/ocaml/libs/eventchn/xeneventchn_stubs.c b/tools/ocaml/libs/eventchn/xeneventchn_stubs.c
index 34dcfed30275..1980acf405a5 100644
--- a/tools/ocaml/libs/eventchn/xeneventchn_stubs.c
+++ b/tools/ocaml/libs/eventchn/xeneventchn_stubs.c
@@ -63,6 +63,8 @@  CAMLprim value stub_eventchn_init(value cloexec)
 	if ( !Bool_val(cloexec) )
 		flags |= XENEVTCHN_NO_CLOEXEC;
 
+	result = caml_alloc_custom(&xenevtchn_ops, sizeof(xce), 0, 1);
+
 	caml_enter_blocking_section();
 	xce = xenevtchn_open(NULL, flags);
 	caml_leave_blocking_section();
@@ -70,7 +72,6 @@  CAMLprim value stub_eventchn_init(value cloexec)
 	if (xce == NULL)
 		caml_failwith("open failed");
 
-	result = caml_alloc_custom(&xenevtchn_ops, sizeof(xce), 0, 1);
 	*(xenevtchn_handle **)Data_custom_val(result) = xce;
 
 	CAMLreturn(result);
@@ -82,6 +83,8 @@  CAMLprim value stub_eventchn_fdopen(value fdval)
 	CAMLlocal1(result);
 	xenevtchn_handle *xce;
 
+	result = caml_alloc_custom(&xenevtchn_ops, sizeof(xce), 0, 1);
+
 	caml_enter_blocking_section();
 	xce = xenevtchn_fdopen(NULL, Int_val(fdval), 0);
 	caml_leave_blocking_section();
@@ -89,7 +92,6 @@  CAMLprim value stub_eventchn_fdopen(value fdval)
 	if (xce == NULL)
 		caml_failwith("evtchn fdopen failed");
 
-	result = caml_alloc_custom(&xenevtchn_ops, sizeof(xce), 0, 1);
 	*(xenevtchn_handle **)Data_custom_val(result) = xce;
 
 	CAMLreturn(result);
diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c b/tools/ocaml/libs/xc/xenctrl_stubs.c
index f9006c662382..ed1cbafdb488 100644
--- a/tools/ocaml/libs/xc/xenctrl_stubs.c
+++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
@@ -98,6 +98,8 @@  CAMLprim value stub_xc_interface_open(value unit)
 	CAMLlocal1(result);
 	xc_interface *xch;
 
+	result = caml_alloc_custom(&xenctrl_ops, sizeof(xch), 0, 1);
+
 	caml_enter_blocking_section();
 	xch = xc_interface_open(NULL, NULL, 0);
 	caml_leave_blocking_section();
@@ -105,7 +107,6 @@  CAMLprim value stub_xc_interface_open(value unit)
 	if ( !xch )
 		failwith_xc(xch);
 
-	result = caml_alloc_custom(&xenctrl_ops, sizeof(xch), 0, 1);
 	*(xc_interface **)Data_custom_val(result) = xch;
 
 	CAMLreturn(result);