[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[xen stable-4.17] tools/oxenstored: Rework Domain evtchn handling to use port_pair



commit 4b418768ef4d75d0f70e4ce7cb5710404527bf47
Author:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
AuthorDate: Wed Nov 30 11:59:34 2022 +0000
Commit:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CommitDate: Tue Dec 20 13:13:40 2022 +0000

    tools/oxenstored: Rework Domain evtchn handling to use port_pair
    
    Inter-domain event channels are always a pair of local and remote ports.
    Right now the handling is asymmetric, caused by the fact that the evtchn is
    bound after the associated Domain object is constructed.
    
    First, move binding of the event channel into the Domain.make() constructor.
    This means the local port no longer needs to be an option.  It also removes
    the final callers of Domain.bind_interdomain.
    
    Next, introduce a new port_pair type to encapsulate the fact that these two
    should be updated together, and replace the previous port and remote_port
    fields.  This refactoring also changes the Domain.get_port interface 
(removing
    an option) so take the opportunity to name it get_local_port instead.
    
    Also, this fixes a use-after-free risk with Domain.close.  Once the evtchn 
has
    been unbound, the same local port number can be reused for a different
    purpose, so explicitly invalidate the ports to prevent their accidental 
misuse
    in the future.
    
    This also cleans up some of the debugging, to always print a port pair.
    
    Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Reviewed-by: Edwin Török <edvin.torok@xxxxxxxxxx>
    Acked-by: Christian Lindig <christian.lindig@xxxxxxxxxx>
    (cherry picked from commit df2db174b36eba67c218763ef621c67912202fc6)
---
 tools/ocaml/xenstored/connections.ml |  9 +----
 tools/ocaml/xenstored/domain.ml      | 75 ++++++++++++++++++------------------
 tools/ocaml/xenstored/domains.ml     |  2 -
 3 files changed, 39 insertions(+), 47 deletions(-)

diff --git a/tools/ocaml/xenstored/connections.ml 
b/tools/ocaml/xenstored/connections.ml
index 7d68c583b4..a80ae0bed2 100644
--- a/tools/ocaml/xenstored/connections.ml
+++ b/tools/ocaml/xenstored/connections.ml
@@ -48,9 +48,7 @@ let add_domain cons dom =
        let xbcon = Xenbus.Xb.open_mmap ~capacity (Domain.get_interface dom) 
(fun () -> Domain.notify dom) in
        let con = Connection.create xbcon (Some dom) in
        Hashtbl.add cons.domains (Domain.get_id dom) con;
-       match Domain.get_port dom with
-       | Some p -> Hashtbl.add cons.ports p con;
-       | None -> ()
+       Hashtbl.add cons.ports (Domain.get_local_port dom) con
 
 let select ?(only_if = (fun _ -> true)) cons =
        Hashtbl.fold (fun _ con (ins, outs) ->
@@ -97,10 +95,7 @@ let del_domain cons id =
                let con = find_domain cons id in
                Hashtbl.remove cons.domains id;
                (match Connection.get_domain con with
-                | Some d ->
-                  (match Domain.get_port d with
-                   | Some p -> Hashtbl.remove cons.ports p
-                   | None -> ())
+                | Some d -> Hashtbl.remove cons.ports (Domain.get_local_port d)
                 | None -> ());
                del_watches cons con;
                Connection.close con
diff --git a/tools/ocaml/xenstored/domain.ml b/tools/ocaml/xenstored/domain.ml
index d59a9401e2..481e10794d 100644
--- a/tools/ocaml/xenstored/domain.ml
+++ b/tools/ocaml/xenstored/domain.ml
@@ -19,14 +19,31 @@ open Printf
 let debug fmt = Logging.debug "domain" fmt
 let warn  fmt = Logging.warn  "domain" fmt
 
+(* A bound inter-domain event channel port pair.  The remote port, and the
+   local port it is bound to. *)
+type port_pair =
+{
+       local: Xeneventchn.t;
+       remote: int;
+}
+
+(* Sentinal port_pair with both set to EVTCHN_INVALID *)
+let invalid_ports =
+{
+       local = Xeneventchn.of_int 0;
+       remote = 0
+}
+
+let string_of_port_pair p =
+       sprintf "(l %d, r %d)" (Xeneventchn.to_int p.local) p.remote
+
 type t =
 {
        id: Xenctrl.domid;
        mfn: nativeint;
        interface: Xenmmap.mmap_interface;
        eventchn: Event.t;
-       mutable remote_port: int;
-       mutable port: Xeneventchn.t option;
+       mutable ports: port_pair;
        mutable bad_client: bool;
        mutable io_credit: int; (* the rounds of ring process left to do, 
default is 0,
                                   usually set to 1 when there is work 
detected, could
@@ -41,8 +58,8 @@ let is_dom0 d = d.id = 0
 let get_id domain = domain.id
 let get_interface d = d.interface
 let get_mfn d = d.mfn
-let get_remote_port d = d.remote_port
-let get_port d = d.port
+let get_remote_port d = d.ports.remote
+let get_local_port d = d.ports.local
 
 let is_bad_domain domain = domain.bad_client
 let mark_as_bad domain = domain.bad_client <- true
@@ -56,54 +73,36 @@ let is_paused_for_conflict dom = dom.conflict_credit <= 0.0
 
 let is_free_to_conflict = is_dom0
 
-let string_of_port = function
-       | None -> "None"
-       | Some x -> string_of_int (Xeneventchn.to_int x)
-
 let dump d chan =
-       fprintf chan "dom,%d,%nd,%d\n" d.id d.mfn d.remote_port
+       fprintf chan "dom,%d,%nd,%d\n" d.id d.mfn d.ports.remote
 
 let rebind_evtchn d remote_port =
-       begin match d.port with
-       | None -> ()
-       | Some p -> Event.unbind d.eventchn p
-       end;
+       Event.unbind d.eventchn d.ports.local;
        let local = Event.bind_interdomain d.eventchn d.id remote_port in
-       debug "domain %d rebind (l %s, r %d) => (l %d, r %d)"
-             d.id (string_of_port d.port) d.remote_port
-             (Xeneventchn.to_int local) remote_port;
-       d.remote_port <- remote_port;
-       d.port <- Some (local)
+       let new_ports = { local; remote = remote_port } in
+       debug "domain %d rebind %s => %s"
+             d.id (string_of_port_pair d.ports) (string_of_port_pair 
new_ports);
+       d.ports <- new_ports
 
 let notify dom =
-       match dom.port with
-       | None -> warn "domain %d: attempt to notify on unknown port" dom.id
-       | Some port -> Event.notify dom.eventchn port
-
-let bind_interdomain dom =
-       begin match dom.port with
-       | None -> ()
-       | Some port -> Event.unbind dom.eventchn port
-       end;
-       dom.port <- Some (Event.bind_interdomain dom.eventchn dom.id 
dom.remote_port);
-       debug "bound domain %d remote port %d to local port %s" dom.id 
dom.remote_port (string_of_port dom.port)
-
+       Event.notify dom.eventchn dom.ports.local
 
 let close dom =
-       debug "domain %d unbound port %s" dom.id (string_of_port dom.port);
-       begin match dom.port with
-       | None -> ()
-       | Some port -> Event.unbind dom.eventchn port
-       end;
+       debug "domain %d unbind %s" dom.id (string_of_port_pair dom.ports);
+       Event.unbind dom.eventchn dom.ports.local;
+       dom.ports <- invalid_ports;
        Xenmmap.unmap dom.interface
 
-let make id mfn remote_port interface eventchn = {
+let make id mfn remote_port interface eventchn =
+       let local = Event.bind_interdomain eventchn id remote_port in
+       let ports = { local; remote = remote_port } in
+       debug "domain %d bind %s" id (string_of_port_pair ports);
+{
        id = id;
        mfn = mfn;
-       remote_port = remote_port;
+       ports;
        interface = interface;
        eventchn = eventchn;
-       port = None;
        bad_client = false;
        io_credit = 0;
        conflict_credit = !Define.conflict_burst_limit;
diff --git a/tools/ocaml/xenstored/domains.ml b/tools/ocaml/xenstored/domains.ml
index 26018ac0dd..2ab0c5f4d8 100644
--- a/tools/ocaml/xenstored/domains.ml
+++ b/tools/ocaml/xenstored/domains.ml
@@ -126,7 +126,6 @@ let create doms domid mfn remote_port =
        let interface = Xenctrl.map_foreign_range xc domid 
(Xenmmap.getpagesize()) mfn in
        let dom = Domain.make domid mfn remote_port interface doms.eventchn in
        Hashtbl.add doms.table domid dom;
-       Domain.bind_interdomain dom;
        dom
 
 let xenstored_kva = ref ""
@@ -144,7 +143,6 @@ let create0 doms =
 
        let dom = Domain.make 0 Nativeint.zero remote_port interface 
doms.eventchn in
        Hashtbl.add doms.table 0 dom;
-       Domain.bind_interdomain dom;
        Domain.notify dom;
        dom
 
--
generated by git-patchbot for /home/xen/git/xen.git#stable-4.17



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.