oxenstored: Protect oxenstored from malicious domains.

add check logic when read from IO ring, and if error happens,
then mark the reading connection as "bad", Unless vm reboot,
oxenstored will not handle message from this connection any more.

xs_ring_stubs.c: add a more strict check on ring reading
connection.ml, domain.ml: add getter and setter for bad flag
process.ml: if exception raised when reading from domain's ring,
            mark this domain as "bad"
xenstored.ml: if a domain is marked as "bad", do not handle it.

Signed-off-by: John Liu <john.liuqiming@huawei.com>
Acked-by: David Scott <dave.scott@eu.citrix.com>

diff --git a/tools/ocaml/libs/xb/xs_ring_stubs.c b/tools/ocaml/libs/xb/xs_ring_stubs.c
index fdd9983d1a309ca439d8f6c5b0aaecac1771aaca..8bd10474903a8d6a8ed125e6fd161806fa326ec7 100644 (file)
--- a/tools/ocaml/libs/xb/xs_ring_stubs.c
+++ b/tools/ocaml/libs/xb/xs_ring_stubs.c
@@ -45,6 +45,10 @@ static int xs_ring_read(struct mmap_interface *interface,
        cons = *(volatile uint32*)&intf->req_cons;
        prod = *(volatile uint32*)&intf->req_prod;
        xen_mb();
+
+       if ((prod - cons) > XENSTORE_RING_SIZE)
+           return -1;
+
        if (prod == cons)
                return 0;
        cons = MASK_XENSTORE_IDX(cons);
@@ -94,7 +98,7 @@ CAMLprim value ml_interface_read(value interface, value buffer, value len)
        res = xs_ring_read(GET_C_STRUCT(interface),
                           String_val(buffer), Int_val(len));
        if (res == -1)
-               caml_failwith("huh");
+               caml_failwith("bad connection");
        result = Val_int(res);
        CAMLreturn(result);
 }

diff --git a/tools/ocaml/xenstored/connection.ml b/tools/ocaml/xenstored/connection.ml
index 32e2f2e35d625afa4d37470e533458cd2f72c23a..273fe4dc1108da4d8af07128319d8fd87f6e6aa3 100644 (file)
--- a/tools/ocaml/xenstored/connection.ml
+++ b/tools/ocaml/xenstored/connection.ml
@@ -38,6 +38,11 @@ and t = {
        mutable perm: Perms.Connection.t;
 }
 
+let mark_as_bad con =
+       match con.dom with
+       |None -> ()
+       | Some domain -> Domain.mark_as_bad domain
+
 let get_path con =
 Printf.sprintf "/local/domain/%i/" (match con.dom with None -> 0 | Some d -> Domain.get_id d)
 

diff --git a/tools/ocaml/xenstored/domain.ml b/tools/ocaml/xenstored/domain.ml
index 85ab282c545616f8a24bb1d14445272e0cfc53b9..444069de767338dd8bc7bc48c2cbd3143acc841a 100644 (file)
--- a/tools/ocaml/xenstored/domain.ml
+++ b/tools/ocaml/xenstored/domain.ml
@@ -27,6 +27,7 @@ type t =
        interface: Xenmmap.mmap_interface;
        eventchn: Event.t;
        mutable port: Xeneventchn.t option;
+       mutable bad_client: bool;
 }
 
 let get_path dom = "/local/domain/" ^ (sprintf "%u" dom.id)
@@ -35,6 +36,9 @@ let get_interface d = d.interface
 let get_mfn d = d.mfn
 let get_remote_port d = d.remote_port
 
+let is_bad_domain domain = domain.bad_client
+let mark_as_bad domain = domain.bad_client <- true
+
 let string_of_port = function
 | None -> "None"
 | Some x -> string_of_int (Xeneventchn.to_int x)
@@ -68,7 +72,8 @@ let make id mfn remote_port interface eventchn = {
        remote_port = remote_port;
        interface = interface;
        eventchn = eventchn;
-       port = None
+       port = None;
+       bad_client = false
 }
 
 let is_dom0 d = d.id = 0

diff --git a/tools/ocaml/xenstored/process.ml b/tools/ocaml/xenstored/process.ml
index a4ff7412642d22f707082fcae453df524dc43f5c..89db56cd85e1d16063a9631f33c68ea11828e0dd 100644 (file)
--- a/tools/ocaml/xenstored/process.ml
+++ b/tools/ocaml/xenstored/process.ml
@@ -374,7 +374,17 @@ let write_answer_log ~ty ~tid ~con ~data =
        Logging.xb_answer ~ty ~tid ~con:(Connection.get_domstr con) data
 
 let do_input store cons doms con =
-       if Connection.do_input con then (
+       let newpacket =
+               try
+                       Connection.do_input con
+               with Failure exp ->
+                       error "caught exception %s" exp;
+                       error "got a bad client %s" (sprintf "%-8s" (Connection.get_domstr con));
+                       Connection.mark_as_bad con;
+                       false
+       in
+
+       if newpacket then (
                let packet = Connection.pop_in con in
                let tid, rid, ty, data = Xenbus.Xb.Packet.unpack packet in
                (* As we don't log IO, do not call an unnecessary sanitize_data 

diff --git a/tools/ocaml/xenstored/xenstored.ml b/tools/ocaml/xenstored/xenstored.ml
index 4045aedc97f725a39f1f04eb91a29bf162e2b76e..438ecb9599159d925ebbec52c85167a0f9a479a0 100644 (file)
--- a/tools/ocaml/xenstored/xenstored.ml
+++ b/tools/ocaml/xenstored/xenstored.ml
@@ -50,9 +50,10 @@ let process_connection_fds store cons domains rset wset =
 
 let process_domains store cons domains =
        let do_io_domain domain =
-               let con = Connections.find_domain cons (Domain.get_id domain) in
-               Process.do_input store cons domains con;
-               Process.do_output store cons domains con in
+               if not (Domain.is_bad_domain domain) then
+                       let con = Connections.find_domain cons (Domain.get_id domain) in
+                               Process.do_input store cons domains con;
+                               Process.do_output store cons domains con in
        Domains.iter domains do_io_domain
 
 let sigusr1_handler store =