From nobody Thu Apr 18 23:44:52 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) client-ip=192.237.175.120; envelope-from=xen-devel-bounces@lists.xenproject.org; helo=lists.xenproject.org; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org; dmarc=pass(p=reject dis=none) header.from=citrix.com ARC-Seal: i=1; a=rsa-sha256; t=1669242379; cv=none; d=zohomail.com; s=zohoarc; b=n0yEgoKCrCDT9LeTgTKGdVfgCv3voX/ZbinZDeGVhcwWw/vQMDp+6D6y+fwNR+TGUss5VhZr6asgs7l8WsdWVB0mn8ZasCSNHN/G3dcFIFoIcgsKANKZuVvMa0zX76AFz4dtnGOzcz/BSyIrPXZPWrOecuHG17TStno3vOR+Hyk= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1669242379; h=Content-Type:Cc:Date:From:List-Subscribe:List-Post:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Sender:Subject:To; bh=z8KXdnui/l9jUc4HzPHR2CvU0Pu6048BlVkKwQMjafU=; b=Ucz47WWrRm7CXk8CWeJVNWKKvoZ7AL+hgkLUt3s7AnDvBaCar6plch9Sq1VJ7KKPh/f7H89AJKzopuAiS6z8JQGK+xawFiIO3YJ2VsKJQdG2+ps2YovKbrn5JtLlImHBTknVOnm9YyuKwWz3J+JrBXmLlDqkysUIYssggkZV5yI= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org; dmarc=pass header.from= (p=reject dis=none) Return-Path: Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) by mx.zohomail.com with SMTPS id 1669242379046747.9072173290484; Wed, 23 Nov 2022 14:26:19 -0800 (PST) Received: from list by lists.xenproject.org with outflank-mailman.447736.704303 (Exim 4.92) (envelope-from ) id 1oxyBW-0003ZR-Eo; Wed, 23 Nov 2022 22:25:38 +0000 Received: by outflank-mailman (output) from mailman id 447736.704303; Wed, 23 Nov 2022 22:25:38 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1oxyBW-0003ZK-CI; Wed, 23 Nov 2022 22:25:38 +0000 Received: by outflank-mailman (input) for mailman id 447736; Wed, 23 Nov 2022 22:25:37 +0000 Received: from se1-gles-flk1-in.inumbo.com ([94.247.172.50] helo=se1-gles-flk1.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1oxyBV-0003ZB-PC for xen-devel@lists.xenproject.org; Wed, 23 Nov 2022 22:25:37 +0000 Received: from esa4.hc3370-68.iphmx.com (esa4.hc3370-68.iphmx.com [216.71.155.144]) by se1-gles-flk1.inumbo.com (Halon) with ESMTPS id be43bc55-6b7d-11ed-8fd2-01056ac49cbb; Wed, 23 Nov 2022 23:25:34 +0100 (CET) X-Outflank-Mailman: Message body and most headers restored to incoming version X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: be43bc55-6b7d-11ed-8fd2-01056ac49cbb DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=citrix.com; s=securemail; t=1669242334; h=from:to:cc:subject:date:message-id:mime-version; bh=ss1D4ingsK86pl+VHuAvo8YZPZvL2G85Rxu9B0/DrhI=; b=DzH5TA7x0x6i5kKGhEZWxjQGQT50DnWt/xswhUgcEclt/AjU6CLC9CTu r5TYbf23UUs4fGfuPf7usi5JJ66PwRF1ZvQZfmInOftvPQppBmvmrVHoD j9mCx6T4cRZnJnbVTX7CNklR/aapSQNi+o9q8Kdah3fC0a4G8YHFENJ3n Y=; Authentication-Results: esa4.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none X-SBRS: None X-MesageID: 87991771 X-Ironport-Server: esa4.hc3370-68.iphmx.com X-Remote-IP: 162.221.156.83 X-Policy: $RELAYED IronPort-Data: A9a23:yQxpZKkO2FyqLJGAzT2usY3o5gziJkRdPkR7XQ2eYbSJt1+Wr1Gzt xIXWmqAOKncMGb3Lt0ga4u3/ElUvJeDzNBgSQc4/y4xFyMWpZLJC+rCIxarNUt+DCFhoGFPt JxCN4aafKjYaleG+39B55C49SEUOZmgH+a6U6icf3grHmeIcQ954Tp7gek1n4V0ttawBgKJq LvartbWfVSowFaYCEpNg064gE4p7aqaVA8w5ARkP6kS5gSGzBH5MbpETU2PByqgKmVrNrbSq 9brlNmR4m7f9hExPdKp+p6TnpoiG+O60aCm0xK6aoD66vRwjnVaPpUTbZLwXXx/mTSR9+2d/ f0W3XCGpaXFCYWX8AgVe0Ew/yiTpsSq8pefSZS0mZT7I0Er7xIAahihZa07FdRwxwp5PY1B3 dsVDh0SdxGqvMez/ILqa7hDiNgyAeC+aevzulk4pd3YJfMvQJSFSKTW/95Imjw3g6iiH96HO ZBfM2A2Kk2dPVsfYT/7C7pn9AusrlD5fydVtxS+oq0v7nKI5AdwzKLsIJzefdniqcB9zhnJ9 zuZoD2R7hcyF+zB5Cqf+2CQlMjsliD9aIM2DYyWz6s/6LGU7jNKU0BHPbehmtG1g1Czc8hSI EsV/mwpt6da3FymSJzxUgO1pFaAvwUAQJxAHusi8gaPx6HIpQGDCQA5oiVpMYJ88pVsHHpzi wHPz4iB6SFTXKO9bn+726iNrBqJHC0pHT4jNAAKURooyoy2yG0stS7nQtFmGa+zq9T6HzDs3 jyHxBQDa6UvYd0jjPviow2e6964jt2QF1NuuF2LNo6wxlkhDLNJcbBE/rQyARxoCI+CBmeMs 3Ef8yR1xLBfVMrd/MBhrQhkIV1I2xpnGGeE6bKMN8N7n9hIx0NPhagKvFlDyL5Ba67puVbBO Sc/Qz956p5JJ2eNZqRqeY+3AMlC5fG+S4W/DKCIPooVOMAZmOq7EMZGPB744owQuBJ0zfFX1 WmzL65A8kr2+Yw4lWHrFo/xIJcgxzwkxHO7eHwI503P7FdfDVbLIYo43KymNLpmtvPd+F2Nm zudXuPToyhivCTFSnG/2eYuwZoidxDX2bieRxRrS9O+ IronPort-HdrOrdr: A9a23:zOFf/KvkWEaXFsm62Gi8xp4w7skDYtV00zEX/kB9WHVpmszxra +TdZUgpHvJYVkqOU3I9ersBEDiewK4yXcW2+ks1N6ZNWGM0ldARLsSj7cKqAePJ8SRzIJgPN 9bAstDNOE= X-IronPort-AV: E=Sophos;i="5.96,187,1665460800"; d="scan'208";a="87991771" From: Andrew Cooper To: Xen-devel CC: Andrew Cooper , Christian Lindig , David Scott , Edwin Torok , Rob Hoes Subject: [PATCH] tools/ocaml/xenctrl: OCaml 5 support, fix use-after-free Date: Wed, 23 Nov 2022 22:25:17 +0000 Message-ID: <20221123222517.12140-1-andrew.cooper3@citrix.com> X-Mailer: git-send-email 2.11.0 MIME-Version: 1.0 X-ZohoMail-DKIM: pass (identity @citrix.com) X-ZM-MESSAGEID: 1669242380460100001 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" The binding for xc_interface_close() free the underlying handle while leavi= ng the Ocaml object still in scope and usable. This would make it easy to suf= fer a use-after-free, if it weren't for the fact that the typical usage is as a singleton that lives for the lifetime of the program. Ocaml 5 no longer permits storing a naked C pointer in an Ocaml value. Therefore, use a Custom block. This allows us to use the finaliser callback to call xc_interface_close(), if the Ocaml object goes out of scope. Signed-off-by: Andrew Cooper Acked-by: Christian Lindig --- CC: Christian Lindig CC: David Scott CC: Edwin Torok CC: Rob Hoes I've confirmed that Xenctrl.close_handle does cause the finaliser to be called, simply by dropping the handle reference. --- tools/ocaml/libs/xc/xenctrl.ml | 3 +-- tools/ocaml/libs/xc/xenctrl.mli | 1 - tools/ocaml/libs/xc/xenctrl_stubs.c | 43 ++++++++++++++++++++++-----------= ---- 3 files changed, 27 insertions(+), 20 deletions(-) diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml index aa650533f718..4b74e31c75cb 100644 --- a/tools/ocaml/libs/xc/xenctrl.ml +++ b/tools/ocaml/libs/xc/xenctrl.ml @@ -175,7 +175,6 @@ exception Error of string type handle =20 external interface_open: unit -> handle =3D "stub_xc_interface_open" -external interface_close: handle -> unit =3D "stub_xc_interface_close" =20 let handle =3D ref None =20 @@ -183,7 +182,7 @@ let get_handle () =3D !handle =20 let close_handle () =3D match !handle with - | Some h -> handle :=3D None; interface_close h + | Some h -> handle :=3D None | None -> () =20 let with_intf f =3D diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.= mli index 5bf5f5dfea36..ddfe84dc22a9 100644 --- a/tools/ocaml/libs/xc/xenctrl.mli +++ b/tools/ocaml/libs/xc/xenctrl.mli @@ -146,7 +146,6 @@ type shutdown_reason =3D Poweroff | Reboot | Suspend | = Crash | Watchdog | Soft_res exception Error of string type handle external interface_open : unit -> handle =3D "stub_xc_interface_open" -external interface_close : handle -> unit =3D "stub_xc_interface_close" =20 (** [with_intf f] runs [f] with a global handle that is opened on demand * and kept open. Conceptually, a client should use either diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c b/tools/ocaml/libs/xc/xenc= trl_stubs.c index f37848ae0bb3..4e1204085422 100644 --- a/tools/ocaml/libs/xc/xenctrl_stubs.c +++ b/tools/ocaml/libs/xc/xenctrl_stubs.c @@ -37,13 +37,28 @@ =20 #include "mmap_stubs.h" =20 -#define _H(__h) ((xc_interface *)(__h)) +#define _H(__h) (*((xc_interface **)Data_custom_val(__h))) #define _D(__d) ((uint32_t)Int_val(__d)) =20 #ifndef Val_none #define Val_none (Val_int(0)) #endif =20 +static void stub_xenctrl_finalize(value v) +{ + xc_interface_close(_H(v)); +} + +static struct custom_operations xenctrl_ops =3D { + .identifier =3D "xenctrl", + .finalize =3D stub_xenctrl_finalize, + .compare =3D custom_compare_default, /* Can't compare */ + .hash =3D custom_hash_default, /* Can't hash */ + .serialize =3D custom_serialize_default, /* Can't serialize */ + .deserialize =3D custom_deserialize_default, /* Can't deserialize */ + .compare_ext =3D custom_compare_ext_default, /* Can't compare */ +}; + #define string_of_option_array(array, index) \ ((Field(array, index) =3D=3D Val_none) ? NULL : String_val(Field(Field(ar= ray, index), 0))) =20 @@ -70,26 +85,20 @@ static void Noreturn failwith_xc(xc_interface *xch) CAMLprim value stub_xc_interface_open(void) { CAMLparam0(); - xc_interface *xch; - - /* Don't assert XC_OPENFLAG_NON_REENTRANT because these bindings - * do not prevent re-entrancy to libxc */ - xch =3D xc_interface_open(NULL, NULL, 0); - if (xch =3D=3D NULL) - failwith_xc(NULL); - CAMLreturn((value)xch); -} - - -CAMLprim value stub_xc_interface_close(value xch) -{ - CAMLparam1(xch); + CAMLlocal1(result); + xc_interface *xch; =20 caml_enter_blocking_section(); - xc_interface_close(_H(xch)); + xch =3D xc_interface_open(NULL, NULL, 0); caml_leave_blocking_section(); =20 - CAMLreturn(Val_unit); + if ( !xch ) + failwith_xc(xch); + + result =3D caml_alloc_custom(&xenctrl_ops, sizeof(xch), 0, 1); + _H(result) =3D xch; + + CAMLreturn(result); } =20 static void domain_handle_of_uuid_string(xen_domain_handle_t h, --=20 2.11.0