CA-391656: OCaml C stubs: release runtime lock around all xenctrl calls

.clang-format

@@ -0,0 +1,9 @@
+BasedOnStyle: GNU
+IndentWidth: 4
+
+# override GNU to match Xen ../../CODING_STYLE more closely
+AlwaysBreakAfterDefinitionReturnType: None
+AlwaysBreakAfterReturnType: None
+SpacesInConditionalStatement: true
+SpaceBeforeParens: ControlStatements
+BreakBeforeBraces: Allman

Makefile

@@ -145,6 +145,7 @@ doc-json:
 
 format:
 	dune build @fmt --auto-promote
+	git ls-files '*.c' '*.h' | xargs clang-format -i
 
 .PHONY: quality-gate
 quality-gate:

ocaml/xenopsd/c_stubs/xenctrlext_stubs.c

@@ -117,7 +117,9 @@ CAMLprim value stub_xenctrlext_get_runstate_info(value xch_val, value domid)
 	int retval;
 	xc_interface *xch = xch_of_val(xch_val);
 
+	caml_enter_blocking_section();
 	retval = xc_get_runstate_info(xch, Int_val(domid), &info);
+	caml_leave_blocking_section();
 	if (retval < 0)
 		failwith_xc(xch);
 
@@ -153,7 +155,9 @@ CAMLprim value stub_xenctrlext_get_boot_cpufeatures(value xch_val)
 	int ret;
 	xc_interface *xch = xch_of_val(xch_val);
 
+	caml_enter_blocking_section();
 	ret = xc_get_boot_cpufeatures(xch, &a, &b, &c, &d, &e, &f, &g, &h);
+	caml_leave_blocking_section();
 	if (ret < 0)
 	  failwith_xc(xch);
 
@@ -191,7 +195,9 @@ CAMLprim value stub_xenctrlext_domain_get_acpi_s_state(value xch_val, value domi
 	int ret;
 	xc_interface* xch = xch_of_val(xch_val);
 
+	caml_enter_blocking_section();
 	ret = xc_get_hvm_param(xch, Int_val(domid), HVM_PARAM_ACPI_S_STATE, &v);
+	caml_leave_blocking_section();
 	if (ret != 0)
 		failwith_xc(xch);
 
@@ -203,7 +209,9 @@ CAMLprim value stub_xenctrlext_domain_send_s3resume(value xch_val, value domid)
 	CAMLparam2(xch_val, domid);
 	xc_interface *xch = xch_of_val(xch_val);
 
+	caml_enter_blocking_section();
 	xcext_domain_send_s3resume(xch, Int_val(domid));
+	caml_leave_blocking_section();
 	CAMLreturn(Val_unit);
 }
 
@@ -213,7 +221,9 @@ CAMLprim value stub_xenctrlext_domain_set_timer_mode(value xch_val, value id, va
 	int ret;
 	xc_interface* xch = xch_of_val(xch_val);
 
+	caml_enter_blocking_section();
 	ret = xcext_domain_set_timer_mode(xch, Int_val(id), Int_val(mode));
+	caml_leave_blocking_section();
 	if (ret < 0)
 		failwith_xc(xch);
 	CAMLreturn(Val_unit);
@@ -243,7 +253,9 @@ CAMLprim value stub_xenctrlext_domain_set_target(value xch_val,
 	CAMLparam3(xch_val, domid, target);
 	xc_interface* xch = xch_of_val(xch_val);
 
+	caml_enter_blocking_section();
 	int retval = xc_domain_set_target(xch, Int_val(domid), Int_val(target));
+	caml_leave_blocking_section();
 	if (retval)
 		failwith_xc(xch);
 	CAMLreturn(Val_unit);
@@ -327,7 +339,9 @@ static int get_cpumap_len(value xch_val, value cpumap)
 {
 	xc_interface* xch = xch_of_val(xch_val);
 	int ml_len = Wosize_val(cpumap);
+	caml_enter_blocking_section();
 	int xc_len = xc_get_max_cpus(xch);
+	caml_leave_blocking_section();
 
 	return (ml_len < xc_len ? ml_len : xc_len);
 }
@@ -341,19 +355,23 @@ CAMLprim value stub_xenctrlext_vcpu_setaffinity_soft(value xch_val, value domid,
 	int retval;
 	xc_interface* xch = xch_of_val(xch_val);
 
+	caml_enter_blocking_section();
 	c_cpumap = xc_cpumap_alloc(xch);
+	caml_leave_blocking_section();
 	if (c_cpumap == NULL)
 		failwith_xc(xch);
 
 	for (i=0; i<len; i++) {
 		if (Bool_val(Field(cpumap, i)))
 			c_cpumap[i/8] |= 1 << (i&7);
 	}
+	caml_enter_blocking_section();
 	retval = xc_vcpu_setaffinity(xch, Int_val(domid),
 				     Int_val(vcpu),
 				     NULL, c_cpumap,
 				     XEN_VCPUAFFINITY_SOFT);
 	free(c_cpumap);
+	caml_leave_blocking_section();
 
 	if (retval < 0)
 		failwith_xc(xch);
@@ -371,7 +389,9 @@ CAMLprim value stub_xenctrlext_numainfo(value xch_val)
         int retval;
         xc_interface* xch = xch_of_val(xch_val);
 
+        caml_enter_blocking_section();
         retval = xc_numainfo(xch, &max_nodes, NULL, NULL);
+        caml_leave_blocking_section();
         if (retval < 0)
             failwith_xc(xch);
 
@@ -383,7 +403,9 @@ CAMLprim value stub_xenctrlext_numainfo(value xch_val)
             caml_raise_out_of_memory();
         }
 
+        caml_enter_blocking_section();
         retval = xc_numainfo(xch, &max_nodes, meminfo, distance);
+        caml_leave_blocking_section();
         if (retval < 0) {
             free(meminfo);
             free(distance);
@@ -425,15 +447,19 @@ CAMLprim value stub_xenctrlext_cputopoinfo(value xch_val)
         int retval;
         xc_interface* xch = xch_of_val(xch_val);
 
+        caml_enter_blocking_section();
         retval = xc_cputopoinfo(xch, &max_cpus, NULL);
+        caml_leave_blocking_section();
         if (retval < 0)
             failwith_xc(xch);
 
         cputopo = calloc(max_cpus, sizeof(*cputopo));
         if (!cputopo)
             caml_raise_out_of_memory();
 
+        caml_enter_blocking_section();
         retval = xc_cputopoinfo(xch, &max_cpus, cputopo);
+        caml_leave_blocking_section();
         if (retval < 0) {
             free(cputopo);
             failwith_xc(xch);
@@ -553,7 +579,9 @@ CAMLprim value stub_xenforeignmemory_open(value unit)
         result = caml_alloc(1, Abstract_tag);
 
         // use NULL instead of a xentoollog handle as those bindings are flawed
+        caml_enter_blocking_section();
         fmem = xenforeignmemory_open(NULL, 0);
+        caml_leave_blocking_section();
 
         if(fmem == NULL) {
                 caml_failwith("Error when opening foreign memory handle");
@@ -568,13 +596,16 @@ CAMLprim value stub_xenforeignmemory_close(value fmem)
 {
         CAMLparam1(fmem);
         int retval;
+        struct xenforeignmemory_handle *handle = Xfm_val(fmem);
 
-        if(Xfm_val(fmem) == NULL) {
+        if(handle == NULL) {
                 caml_invalid_argument(
                         "Error: cannot close NULL foreign memory handle");
         }
 
-        retval = xenforeignmemory_close(Xfm_val(fmem));
+        caml_enter_blocking_section();
+        retval = xenforeignmemory_close(handle);
+        caml_leave_blocking_section();
 
         if(retval < 0) {
                 caml_failwith("Error when closing foreign memory handle");
@@ -632,11 +663,12 @@ CAMLprim value stub_xenforeignmemory_map(value fmem, value dom,
                 cell = Field(cell, 1);
         }
 
+        caml_enter_blocking_section();
         retval = xenforeignmemory_map
                 (handle, Int_val(dom), prot, pages_length, arr, NULL);
         the_errno = errno;
-
         free(arr);
+        caml_leave_blocking_section();
 
         if(retval == NULL) {
                 raise_unix_errno_msg(the_errno,
@@ -655,12 +687,15 @@ CAMLprim value stub_xenforeignmemory_unmap(value fmem, value mapping)
         CAMLparam2(fmem, mapping);
         size_t pages;
         int retval, the_errno;
+        struct xenforeignmemory_handle *handle = Xfm_val(fmem);
+        void *data = Caml_ba_data_val(mapping);
 
         // convert mapping to pages and addr
         pages = Caml_ba_array_val(mapping)->dim[0] / 4096;
 
-        retval = xenforeignmemory_unmap(Xfm_val(fmem),
-                        Caml_ba_data_val(mapping), pages);
+        caml_enter_blocking_section();
+        retval = xenforeignmemory_unmap(handle, data, pages);
+        caml_leave_blocking_section();
         the_errno = errno;
 
         if(retval < 0) {

unixpwd/c/unixpwd_stubs.c

@@ -20,102 +20,88 @@
 #include <caml/fail.h>
 #include <caml/callback.h>
 #include <caml/memory.h>
+#include <caml/threads.h>
+#include <caml/unixsupport.h>
 
 #include "unixpwd.h"
 
-
-CAMLprim        value
-caml_unixpwd_getpwd(value caml_user)
+static CAMLprim value caml_unixpwd_get_(value caml_user, const char *fname, char*(*f)(const char*))
 {
     CAMLparam1(caml_user);
-    const char     *user;
-    char           *passwd;
+    char     *user;
+    char     *passwd;
+    int       saved_errno;
     CAMLlocal1(pw);
 
-    user = String_val(caml_user);
-    passwd = unixpwd_getpwd(user);
-    if (passwd == NULL && errno != 0)
-        caml_failwith(strerror(errno));
-    if (passwd == NULL)
-        caml_failwith("unspecified error in caml_unixpwd_getpwd()");
+    user = caml_stat_strdup(String_val(caml_user));
+    caml_enter_blocking_section();
+    errno = 0;
+    passwd = f(user);
+    saved_errno = errno;
+    caml_stat_free(user); user = NULL;
+    caml_leave_blocking_section();
+    errno = saved_errno;
+
+    if (passwd == NULL) /* errno of 0 will be mapped to `EUNKNOWNERR of 0` */
+        uerror(fname, caml_user);
 
     pw = caml_copy_string(passwd);
     free(passwd);
     CAMLreturn(pw);
 }
 
 CAMLprim        value
-caml_unixpwd_getspw(value caml_user)
+caml_unixpwd_getpwd(value caml_user)
 {
-    CAMLparam1(caml_user);
-    const char     *user;
-    char           *passwd;
-    CAMLlocal1(pw);
-
-    user = String_val(caml_user);
-    passwd = unixpwd_getspw(user);
-    if (passwd == NULL && errno != 0)
-        caml_failwith(strerror(errno));
-    if (passwd == NULL)
-        caml_failwith("unspecified error in caml_unixpwd_getspw()");
-
-    pw = caml_copy_string(passwd);
-    free(passwd);
-    CAMLreturn(pw);
+    return caml_unixpwd_get_(caml_user, "unixpwd_getpwd", unixpwd_getpwd);
 }
 
-
+CAMLprim        value
+caml_unixpwd_getspw(value caml_user)
+{
+    return caml_unixpwd_get_(caml_user, "unixpwd_getspw", unixpwd_getspw);
+}
 
 CAMLprim        value
 caml_unixpwd_get(value caml_user)
 {
-    CAMLparam1(caml_user);
-    const char     *user;
-    char           *passwd;
-    CAMLlocal1(pw);
-
-    user = String_val(caml_user);
-    passwd = unixpwd_get(user);
-    if (passwd == NULL && errno != 0)
-        caml_failwith(strerror(errno));
-    if (passwd == NULL)
-        caml_failwith("unspecified error in caml_unixpwd_get()");
-
-    pw = caml_copy_string(passwd);
-    free(passwd);
-    CAMLreturn(pw);
+    return caml_unixpwd_get_(caml_user, "unixpwd_get", unixpwd_get);
 }
 
-CAMLprim        value
-caml_unixpwd_setpwd(value caml_user, value caml_password)
+static CAMLprim value caml_unixpwd_set_(value caml_user, value caml_password, const char *fname, int(*f)(const char*, char*))
 {
     CAMLparam2(caml_user, caml_password);
-    const char     *user;
-    char           *password;
-    int             rc;
+    char     *user;
+    char     *password;
+    int       saved_errno;
+    int       rc;
 
-    user = String_val(caml_user);
+    user = caml_stat_strdup(String_val(caml_user));
     password = caml_stat_strdup(String_val(caml_password));
-    rc = unixpwd_setpwd(user, password);
+    caml_enter_blocking_section();
+    errno = 0;
+    rc = f(user, password);
+    saved_errno = errno;
+    caml_leave_blocking_section();
+    caml_stat_free(user);
     caml_stat_free(password);
+    errno = saved_errno;
+
     if (rc != 0)
-        caml_failwith(strerror(rc));
+        uerror(fname, caml_user); /* only raise with user not pass */
     CAMLreturn(Val_unit);
 }
 
 CAMLprim        value
-caml_unixpwd_setspw(value caml_user, value caml_password)
+caml_unixpwd_setpwd(value caml_user, value caml_password)
 {
-    CAMLparam2(caml_user, caml_password);
-    const char     *user;
-    char           *password;
-    int             rc;
+    return caml_unixpwd_set_(caml_user, caml_password, "unixpwd_setpwd",
+                            unixpwd_setpwd);
+}
 
-    user = String_val(caml_user);
-    password = caml_stat_strdup(String_val(caml_password));
-    rc = unixpwd_setspw(user, password);
-    caml_stat_free(password);
-    if (rc != 0)
-        caml_failwith(strerror(rc));
-    CAMLreturn(Val_unit);
+CAMLprim        value
+caml_unixpwd_setspw(value caml_user, value caml_password)
+{
+    return caml_unixpwd_set_(caml_user, caml_password, "unixpwd_setspw",
+                            unixpwd_setspw);
 }