Version in base suite: 4.11.4+37-g3263f257ca-1 Version in overlay suite: 4.11.4+57-g41a822c392-1 Base version: xen_4.11.4+57-g41a822c392-1 Target version: xen_4.11.4+57-g41a822c392-2 Base file: /srv/ftp-master.debian.org/ftp/pool/main/x/xen/xen_4.11.4+57-g41a822c392-1.dsc Target file: /srv/ftp-master.debian.org/policy/pool/main/x/xen/xen_4.11.4+57-g41a822c392-2.dsc changelog | 32 patches/0050-tools-ocaml-xenstored-do-permission-checks-on-xensto.patch | 93 + patches/0051-tools-xenstore-allow-removing-child-of-a-node-exceed.patch | 152 ++ patches/0052-tools-xenstore-ignore-transaction-id-for-un-watch.patch | 82 + patches/0053-tools-xenstore-fix-node-accounting-after-failed-node.patch | 99 + patches/0054-tools-xenstore-simplify-and-rename-check_event_node.patch | 51 patches/0055-tools-xenstore-check-privilege-for-XS_IS_DOMAIN_INTR.patch | 110 ++ patches/0056-tools-xenstore-rework-node-removal.patch | 213 +++ patches/0057-tools-xenstore-fire-watches-only-when-removing-a-spe.patch | 113 ++ patches/0058-tools-xenstore-introduce-node_perms-structure.patch | 285 +++++ patches/0059-tools-xenstore-allow-special-watches-for-privileged-.patch | 232 ++++ patches/0060-tools-xenstore-avoid-watch-events-for-nodes-without-.patch | 370 ++++++ patches/0061-tools-ocaml-xenstored-ignore-transaction-id-for-un-w.patch | 47 patches/0062-tools-ocaml-xenstored-check-privilege-for-XS_IS_DOMA.patch | 34 patches/0063-tools-ocaml-xenstored-unify-watch-firing.patch | 33 patches/0064-tools-ocaml-xenstored-introduce-permissions-for-spec.patch | 124 ++ patches/0065-tools-ocaml-xenstored-avoid-watch-events-for-nodes-w.patch | 399 +++++++ patches/0066-tools-ocaml-xenstored-add-xenstored.conf-flag-to-tur.patch | 91 + patches/0067-tools-xenstore-revoke-access-rights-for-removed-doma.patch | 542 ++++++++++ patches/0068-tools-ocaml-xenstored-clean-up-permissions-for-dead-.patch | 117 ++ patches/0069-tools-ocaml-xenstored-Fix-path-length-validation.patch | 150 ++ patches/0070-tools-xenstore-drop-watch-event-messages-exceeding-m.patch | 53 patches/0071-tools-xenstore-Preserve-bad-client-until-they-are-de.patch | 194 +++ patches/0072-tools-ocaml-xenstored-delete-watch-from-trie-too-whe.patch | 71 + patches/0073-tools-ocaml-xenstored-only-Dom0-can-change-node-owne.patch | 46 patches/0074-x86-avoid-calling-svm-vmx-_do_resume.patch | 188 +++ patches/0075-evtchn-FIFO-re-order-and-synchronize-with-map_contro.patch | 59 + patches/0076-evtchn-FIFO-add-2nd-smp_rmb-to-evtchn_fifo_word_from.patch | 45 patches/series | 27 29 files changed, 4052 insertions(+) diff -Nru xen-4.11.4+57-g41a822c392/debian/changelog xen-4.11.4+57-g41a822c392/debian/changelog --- xen-4.11.4+57-g41a822c392/debian/changelog 2020-12-03 12:56:29.000000000 +0000 +++ xen-4.11.4+57-g41a822c392/debian/changelog 2020-12-11 21:10:09.000000000 +0000 @@ -1,3 +1,35 @@ +xen (4.11.4+57-g41a822c392-2) buster-security; urgency=high + + * Apply security fixes for the following issues: + - oxenstored: permissions not checked on root node + XSA-353 (CVE-2020-29479) + - xenstore watch notifications lacking permission checks + XSA-115 (CVE-2020-29480) + - Xenstore: new domains inheriting existing node permissions + XSA-322 (CVE-2020-29481) + - Xenstore: wrong path length check + XSA-323 (CVE-2020-29482) + - Xenstore: guests can crash xenstored via watchs + XSA-324 (CVE-2020-29484) + - Xenstore: guests can disturb domain cleanup + XSA-325 (CVE-2020-29483) + - oxenstored memory leak in reset_watches + XSA-330 (CVE-2020-29485) + - oxenstored: node ownership can be changed by unprivileged clients + XSA-352 (CVE-2020-29486) + - undue recursion in x86 HVM context switch code + XSA-348 (CVE-2020-29566) + - FIFO event channels control block related ordering + XSA-358 (CVE-2020-29570) + - FIFO event channels control structure ordering + XSA-359 (CVE-2020-29571) + * Note that the following XSA are not listed, because... + - XSA-349 and XSA-350 have patches for the Linux kernel + - XSA-354 has patches for the XAPI toolstack + - XSA-356 only applies to Xen 4.14 + + -- Hans van Kranenburg Fri, 11 Dec 2020 22:10:09 +0100 + xen (4.11.4+57-g41a822c392-1) buster-security; urgency=high * Update to new upstream version 4.11.4+57-g41a822c392, which also contains diff -Nru xen-4.11.4+57-g41a822c392/debian/patches/0050-tools-ocaml-xenstored-do-permission-checks-on-xensto.patch xen-4.11.4+57-g41a822c392/debian/patches/0050-tools-ocaml-xenstored-do-permission-checks-on-xensto.patch --- xen-4.11.4+57-g41a822c392/debian/patches/0050-tools-ocaml-xenstored-do-permission-checks-on-xensto.patch 1970-01-01 00:00:00.000000000 +0000 +++ xen-4.11.4+57-g41a822c392/debian/patches/0050-tools-ocaml-xenstored-do-permission-checks-on-xensto.patch 2020-12-11 21:10:09.000000000 +0000 @@ -0,0 +1,93 @@ +From: =?utf-8?b?RWR3aW4gVMO2csO2aw==?= +Date: Fri, 11 Dec 2020 21:48:14 +0100 +Subject: tools/ocaml/xenstored: do permission checks on xenstore root +MIME-Version: 1.0 +Content-Type: text/plain; charset="utf-8" +Content-Transfer-Encoding: 8bit + +This was lacking in a disappointing number of places. + +The xenstore root node is treated differently from all other nodes, because it +doesn't have a parent, and mutation requires changing the parent. + +Unfortunately this lead to open-coding the special case for root into every +single xenstore operation, and out of all the xenstore operations only read +did a permission check when handling the root node. + +This means that an unprivileged guest can: + + * xenstore-chmod / to its liking and subsequently write new arbitrary nodes + there (subject to quota) + * xenstore-rm -r / deletes almost the entire xenstore tree (xenopsd quickly + refills some, but you are left with a broken system) + * DIRECTORY on / lists all children when called through python + bindings (xenstore-ls stops at /local because it tries to list recursively) + * get-perms on / works too, but that is just a minor information leak + +Add the missing permission checks, but this should really be refactored to do +the root handling and permission checks on the node only once from a single +function, instead of getting it wrong nearly everywhere. + +This is XSA-353. + +Signed-off-by: Edwin Török +Acked-by: Christian Lindig +Reviewed-by: Andrew Cooper +--- + tools/ocaml/xenstored/store.ml | 24 ++++++++++++++---------- + 1 file changed, 14 insertions(+), 10 deletions(-) + +diff --git a/tools/ocaml/xenstored/store.ml b/tools/ocaml/xenstored/store.ml +index 5a8c377..6375a1c 100644 +--- a/tools/ocaml/xenstored/store.ml ++++ b/tools/ocaml/xenstored/store.ml +@@ -273,15 +273,17 @@ let path_rm store perm path = + Node.del_childname node name + with Not_found -> + raise Define.Doesnt_exist in +- if path = [] then ++ if path = [] then ( ++ Node.check_perm store.root perm Perms.WRITE; + Node.del_all_children store.root +- else ++ ) else + Path.apply_modify store.root path do_rm + + let path_setperms store perm path perms = +- if path = [] then ++ if path = [] then ( ++ Node.check_perm store.root perm Perms.WRITE; + Node.set_perms store.root perms +- else ++ ) else + let do_setperms node name = + let c = Node.find node name in + Node.check_owner c perm; +@@ -313,9 +315,10 @@ let read store perm path = + + let ls store perm path = + let children = +- if path = [] then +- (Node.get_children store.root) +- else ++ if path = [] then ( ++ Node.check_perm store.root perm Perms.READ; ++ Node.get_children store.root ++ ) else + let do_ls node name = + let cnode = Node.find node name in + Node.check_perm cnode perm Perms.READ; +@@ -324,9 +327,10 @@ let ls store perm path = + List.rev (List.map (fun n -> Symbol.to_string n.Node.name) children) + + let getperms store perm path = +- if path = [] then +- (Node.get_perms store.root) +- else ++ if path = [] then ( ++ Node.check_perm store.root perm Perms.READ; ++ Node.get_perms store.root ++ ) else + let fct n name = + let c = Node.find n name in + Node.check_perm c perm Perms.READ; diff -Nru xen-4.11.4+57-g41a822c392/debian/patches/0051-tools-xenstore-allow-removing-child-of-a-node-exceed.patch xen-4.11.4+57-g41a822c392/debian/patches/0051-tools-xenstore-allow-removing-child-of-a-node-exceed.patch --- xen-4.11.4+57-g41a822c392/debian/patches/0051-tools-xenstore-allow-removing-child-of-a-node-exceed.patch 1970-01-01 00:00:00.000000000 +0000 +++ xen-4.11.4+57-g41a822c392/debian/patches/0051-tools-xenstore-allow-removing-child-of-a-node-exceed.patch 2020-12-11 21:10:09.000000000 +0000 @@ -0,0 +1,152 @@ +From: Juergen Gross +Date: Thu, 11 Jun 2020 16:12:37 +0200 +Subject: tools/xenstore: allow removing child of a node exceeding quota + +An unprivileged user of Xenstore is not allowed to write nodes with a +size exceeding a global quota, while privileged users like dom0 are +allowed to write such nodes. The size of a node is the needed space +to store all node specific data, this includes the names of all +children of the node. + +When deleting a node its parent has to be modified by removing the +name of the to be deleted child from it. + +This results in the strange situation that an unprivileged owner of a +node might not succeed in deleting that node in case its parent is +exceeding the quota of that unprivileged user (it might have been +written by dom0), as the user is not allowed to write the updated +parent node. + +Fix that by not checking the quota when writing a node for the +purpose of removing a child's name only. + +The same applies to transaction handling: a node being read during a +transaction is written to the transaction specific area and it should +not be tested for exceeding the quota, as it might not be owned by +the reader and presumably the original write would have failed if the +node is owned by the reader. + +This is part of XSA-115. + +Signed-off-by: Juergen Gross +Reviewed-by: Julien Grall +Reviewed-by: Paul Durrant +--- + tools/xenstore/xenstored_core.c | 20 +++++++++++--------- + tools/xenstore/xenstored_core.h | 3 ++- + tools/xenstore/xenstored_transaction.c | 2 +- + 3 files changed, 14 insertions(+), 11 deletions(-) + +diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c +index c8e4237..43900a3 100644 +--- a/tools/xenstore/xenstored_core.c ++++ b/tools/xenstore/xenstored_core.c +@@ -417,7 +417,8 @@ static struct node *read_node(struct connection *conn, const void *ctx, + return node; + } + +-int write_node_raw(struct connection *conn, TDB_DATA *key, struct node *node) ++int write_node_raw(struct connection *conn, TDB_DATA *key, struct node *node, ++ bool no_quota_check) + { + TDB_DATA data; + void *p; +@@ -427,7 +428,7 @@ int write_node_raw(struct connection *conn, TDB_DATA *key, struct node *node) + + node->num_perms*sizeof(node->perms[0]) + + node->datalen + node->childlen; + +- if (domain_is_unprivileged(conn) && ++ if (!no_quota_check && domain_is_unprivileged(conn) && + data.dsize >= quota_max_entry_size) { + errno = ENOSPC; + return errno; +@@ -455,14 +456,15 @@ int write_node_raw(struct connection *conn, TDB_DATA *key, struct node *node) + return 0; + } + +-static int write_node(struct connection *conn, struct node *node) ++static int write_node(struct connection *conn, struct node *node, ++ bool no_quota_check) + { + TDB_DATA key; + + if (access_node(conn, node, NODE_ACCESS_WRITE, &key)) + return errno; + +- return write_node_raw(conn, &key, node); ++ return write_node_raw(conn, &key, node, no_quota_check); + } + + static enum xs_perm_type perm_for_conn(struct connection *conn, +@@ -999,7 +1001,7 @@ static struct node *create_node(struct connection *conn, const void *ctx, + /* We write out the nodes down, setting destructor in case + * something goes wrong. */ + for (i = node; i; i = i->parent) { +- if (write_node(conn, i)) { ++ if (write_node(conn, i, false)) { + domain_entry_dec(conn, i); + return NULL; + } +@@ -1039,7 +1041,7 @@ static int do_write(struct connection *conn, struct buffered_data *in) + } else { + node->data = in->buffer + offset; + node->datalen = datalen; +- if (write_node(conn, node)) ++ if (write_node(conn, node, false)) + return errno; + } + +@@ -1115,7 +1117,7 @@ static int remove_child_entry(struct connection *conn, struct node *node, + size_t childlen = strlen(node->children + offset); + memdel(node->children, offset, childlen + 1, node->childlen); + node->childlen -= childlen + 1; +- return write_node(conn, node); ++ return write_node(conn, node, true); + } + + +@@ -1254,7 +1256,7 @@ static int do_set_perms(struct connection *conn, struct buffered_data *in) + node->num_perms = num; + domain_entry_inc(conn, node); + +- if (write_node(conn, node)) ++ if (write_node(conn, node, false)) + return errno; + + fire_watches(conn, in, name, false); +@@ -1514,7 +1516,7 @@ static void manual_node(const char *name, const char *child) + if (child) + node->childlen = strlen(child) + 1; + +- if (write_node(NULL, node)) ++ if (write_node(NULL, node, false)) + barf_perror("Could not create initial node %s", name); + talloc_free(node); + } +diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h +index 3d7eb91..ee31237 100644 +--- a/tools/xenstore/xenstored_core.h ++++ b/tools/xenstore/xenstored_core.h +@@ -151,7 +151,8 @@ void send_ack(struct connection *conn, enum xsd_sockmsg_type type); + char *canonicalize(struct connection *conn, const void *ctx, const char *node); + + /* Write a node to the tdb data base. */ +-int write_node_raw(struct connection *conn, TDB_DATA *key, struct node *node); ++int write_node_raw(struct connection *conn, TDB_DATA *key, struct node *node, ++ bool no_quota_check); + + /* Get this node, checking we have permissions. */ + struct node *get_node(struct connection *conn, +diff --git a/tools/xenstore/xenstored_transaction.c b/tools/xenstore/xenstored_transaction.c +index 75816dd..e505429 100644 +--- a/tools/xenstore/xenstored_transaction.c ++++ b/tools/xenstore/xenstored_transaction.c +@@ -276,7 +276,7 @@ int access_node(struct connection *conn, struct node *node, + i->check_gen = true; + if (node->generation != NO_GENERATION) { + set_tdb_key(trans_name, &local_key); +- ret = write_node_raw(conn, &local_key, node); ++ ret = write_node_raw(conn, &local_key, node, true); + if (ret) + goto err; + i->ta_node = true; diff -Nru xen-4.11.4+57-g41a822c392/debian/patches/0052-tools-xenstore-ignore-transaction-id-for-un-watch.patch xen-4.11.4+57-g41a822c392/debian/patches/0052-tools-xenstore-ignore-transaction-id-for-un-watch.patch --- xen-4.11.4+57-g41a822c392/debian/patches/0052-tools-xenstore-ignore-transaction-id-for-un-watch.patch 1970-01-01 00:00:00.000000000 +0000 +++ xen-4.11.4+57-g41a822c392/debian/patches/0052-tools-xenstore-ignore-transaction-id-for-un-watch.patch 2020-12-11 21:10:09.000000000 +0000 @@ -0,0 +1,82 @@ +From: Juergen Gross +Date: Thu, 11 Jun 2020 16:12:38 +0200 +Subject: tools/xenstore: ignore transaction id for [un]watch + +Instead of ignoring the transaction id for XS_WATCH and XS_UNWATCH +commands as it is documented in docs/misc/xenstore.txt, it is tested +for validity today. + +Really ignore the transaction id for XS_WATCH and XS_UNWATCH. + +This is part of XSA-115. + +Signed-off-by: Juergen Gross +Reviewed-by: Julien Grall +Reviewed-by: Paul Durrant +--- + tools/xenstore/xenstored_core.c | 26 ++++++++++++++++---------- + 1 file changed, 16 insertions(+), 10 deletions(-) + +diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c +index 43900a3..ec06b4e 100644 +--- a/tools/xenstore/xenstored_core.c ++++ b/tools/xenstore/xenstored_core.c +@@ -1268,13 +1268,17 @@ static int do_set_perms(struct connection *conn, struct buffered_data *in) + static struct { + const char *str; + int (*func)(struct connection *conn, struct buffered_data *in); ++ unsigned int flags; ++#define XS_FLAG_NOTID (1U << 0) /* Ignore transaction id. */ + } const wire_funcs[XS_TYPE_COUNT] = { + [XS_CONTROL] = { "CONTROL", do_control }, + [XS_DIRECTORY] = { "DIRECTORY", send_directory }, + [XS_READ] = { "READ", do_read }, + [XS_GET_PERMS] = { "GET_PERMS", do_get_perms }, +- [XS_WATCH] = { "WATCH", do_watch }, +- [XS_UNWATCH] = { "UNWATCH", do_unwatch }, ++ [XS_WATCH] = ++ { "WATCH", do_watch, XS_FLAG_NOTID }, ++ [XS_UNWATCH] = ++ { "UNWATCH", do_unwatch, XS_FLAG_NOTID }, + [XS_TRANSACTION_START] = { "TRANSACTION_START", do_transaction_start }, + [XS_TRANSACTION_END] = { "TRANSACTION_END", do_transaction_end }, + [XS_INTRODUCE] = { "INTRODUCE", do_introduce }, +@@ -1296,7 +1300,7 @@ static struct { + + static const char *sockmsg_string(enum xsd_sockmsg_type type) + { +- if ((unsigned)type < XS_TYPE_COUNT && wire_funcs[type].str) ++ if ((unsigned int)type < ARRAY_SIZE(wire_funcs) && wire_funcs[type].str) + return wire_funcs[type].str; + + return "**UNKNOWN**"; +@@ -1311,7 +1315,14 @@ static void process_message(struct connection *conn, struct buffered_data *in) + enum xsd_sockmsg_type type = in->hdr.msg.type; + int ret; + +- trans = transaction_lookup(conn, in->hdr.msg.tx_id); ++ if ((unsigned int)type >= XS_TYPE_COUNT || !wire_funcs[type].func) { ++ eprintf("Client unknown operation %i", type); ++ send_error(conn, ENOSYS); ++ return; ++ } ++ ++ trans = (wire_funcs[type].flags & XS_FLAG_NOTID) ++ ? NULL : transaction_lookup(conn, in->hdr.msg.tx_id); + if (IS_ERR(trans)) { + send_error(conn, -PTR_ERR(trans)); + return; +@@ -1320,12 +1331,7 @@ static void process_message(struct connection *conn, struct buffered_data *in) + assert(conn->transaction == NULL); + conn->transaction = trans; + +- if ((unsigned)type < XS_TYPE_COUNT && wire_funcs[type].func) +- ret = wire_funcs[type].func(conn, in); +- else { +- eprintf("Client unknown operation %i", type); +- ret = ENOSYS; +- } ++ ret = wire_funcs[type].func(conn, in); + if (ret) + send_error(conn, ret); + diff -Nru xen-4.11.4+57-g41a822c392/debian/patches/0053-tools-xenstore-fix-node-accounting-after-failed-node.patch xen-4.11.4+57-g41a822c392/debian/patches/0053-tools-xenstore-fix-node-accounting-after-failed-node.patch --- xen-4.11.4+57-g41a822c392/debian/patches/0053-tools-xenstore-fix-node-accounting-after-failed-node.patch 1970-01-01 00:00:00.000000000 +0000 +++ xen-4.11.4+57-g41a822c392/debian/patches/0053-tools-xenstore-fix-node-accounting-after-failed-node.patch 2020-12-11 21:10:09.000000000 +0000 @@ -0,0 +1,99 @@ +From: Juergen Gross +Date: Thu, 11 Jun 2020 16:12:39 +0200 +Subject: tools/xenstore: fix node accounting after failed node creation + +When a node creation fails the number of nodes of the domain should be +the same as before the failed node creation. In case of failure when +trying to create a node requiring to create one or more intermediate +nodes as well (e.g. when /a/b/c/d is to be created, but /a/b isn't +existing yet) it might happen that the number of nodes of the creating +domain is not reset to the value it had before. + +So move the quota accounting out of construct_node() and into the node +write loop in create_node() in order to be able to undo the accounting +in case of an error in the intermediate node destructor. + +This is part of XSA-115. + +Signed-off-by: Juergen Gross +Reviewed-by: Paul Durrant +Acked-by: Julien Grall +--- + tools/xenstore/xenstored_core.c | 37 +++++++++++++++++++++++++------------ + 1 file changed, 25 insertions(+), 12 deletions(-) + +diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c +index ec06b4e..444dea3 100644 +--- a/tools/xenstore/xenstored_core.c ++++ b/tools/xenstore/xenstored_core.c +@@ -925,11 +925,6 @@ static struct node *construct_node(struct connection *conn, const void *ctx, + if (!parent) + return NULL; + +- if (domain_entry(conn) >= quota_nb_entry_per_domain) { +- errno = ENOSPC; +- return NULL; +- } +- + /* Add child to parent. */ + base = basename(name); + baselen = strlen(base) + 1; +@@ -962,7 +957,6 @@ static struct node *construct_node(struct connection *conn, const void *ctx, + node->children = node->data = NULL; + node->childlen = node->datalen = 0; + node->parent = parent; +- domain_entry_inc(conn, node); + return node; + + nomem: +@@ -982,6 +976,9 @@ static int destroy_node(void *_node) + key.dsize = strlen(node->name); + + tdb_delete(tdb_ctx, key); ++ ++ domain_entry_dec(talloc_parent(node), node); ++ + return 0; + } + +@@ -998,18 +995,34 @@ static struct node *create_node(struct connection *conn, const void *ctx, + node->data = data; + node->datalen = datalen; + +- /* We write out the nodes down, setting destructor in case +- * something goes wrong. */ ++ /* ++ * We write out the nodes bottom up. ++ * All new created nodes will have i->parent set, while the final ++ * node will be already existing and won't have i->parent set. ++ * New nodes are subject to quota handling. ++ * Initially set a destructor for all new nodes removing them from ++ * TDB again and undoing quota accounting for the case of an error ++ * during the write loop. ++ */ + for (i = node; i; i = i->parent) { +- if (write_node(conn, i, false)) { +- domain_entry_dec(conn, i); ++ /* i->parent is set for each new node, so check quota. */ ++ if (i->parent && ++ domain_entry(conn) >= quota_nb_entry_per_domain) { ++ errno = ENOSPC; + return NULL; + } +- talloc_set_destructor(i, destroy_node); ++ if (write_node(conn, i, false)) ++ return NULL; ++ ++ /* Account for new node, set destructor for error case. */ ++ if (i->parent) { ++ domain_entry_inc(conn, i); ++ talloc_set_destructor(i, destroy_node); ++ } + } + + /* OK, now remove destructors so they stay around */ +- for (i = node; i; i = i->parent) ++ for (i = node; i->parent; i = i->parent) + talloc_set_destructor(i, NULL); + return node; + } diff -Nru xen-4.11.4+57-g41a822c392/debian/patches/0054-tools-xenstore-simplify-and-rename-check_event_node.patch xen-4.11.4+57-g41a822c392/debian/patches/0054-tools-xenstore-simplify-and-rename-check_event_node.patch --- xen-4.11.4+57-g41a822c392/debian/patches/0054-tools-xenstore-simplify-and-rename-check_event_node.patch 1970-01-01 00:00:00.000000000 +0000 +++ xen-4.11.4+57-g41a822c392/debian/patches/0054-tools-xenstore-simplify-and-rename-check_event_node.patch 2020-12-11 21:10:09.000000000 +0000 @@ -0,0 +1,51 @@ +From: Juergen Gross +Date: Thu, 11 Jun 2020 16:12:40 +0200 +Subject: tools/xenstore: simplify and rename check_event_node() + +There is no path which allows to call check_event_node() without a +event name. So don't let the result depend on the name being NULL and +add an assert() covering that case. + +Rename the function to check_special_event() to better match the +semantics. + +This is part of XSA-115. + +Signed-off-by: Juergen Gross +Reviewed-by: Julien Grall +Reviewed-by: Paul Durrant +--- + tools/xenstore/xenstored_watch.c | 12 +++++------- + 1 file changed, 5 insertions(+), 7 deletions(-) + +diff --git a/tools/xenstore/xenstored_watch.c b/tools/xenstore/xenstored_watch.c +index 0dc5a40..4580628 100644 +--- a/tools/xenstore/xenstored_watch.c ++++ b/tools/xenstore/xenstored_watch.c +@@ -47,13 +47,11 @@ struct watch + char *node; + }; + +-static bool check_event_node(const char *node) ++static bool check_special_event(const char *name) + { +- if (!node || !strstarts(node, "@")) { +- errno = EINVAL; +- return false; +- } +- return true; ++ assert(name); ++ ++ return strstarts(name, "@"); + } + + /* Is child a subnode of parent, or equal? */ +@@ -87,7 +85,7 @@ static void add_event(struct connection *conn, + unsigned int len; + char *data; + +- if (!check_event_node(name)) { ++ if (!check_special_event(name)) { + /* Can this conn load node, or see that it doesn't exist? */ + struct node *node = get_node(conn, ctx, name, XS_PERM_READ); + /* diff -Nru xen-4.11.4+57-g41a822c392/debian/patches/0055-tools-xenstore-check-privilege-for-XS_IS_DOMAIN_INTR.patch xen-4.11.4+57-g41a822c392/debian/patches/0055-tools-xenstore-check-privilege-for-XS_IS_DOMAIN_INTR.patch --- xen-4.11.4+57-g41a822c392/debian/patches/0055-tools-xenstore-check-privilege-for-XS_IS_DOMAIN_INTR.patch 1970-01-01 00:00:00.000000000 +0000 +++ xen-4.11.4+57-g41a822c392/debian/patches/0055-tools-xenstore-check-privilege-for-XS_IS_DOMAIN_INTR.patch 2020-12-11 21:10:09.000000000 +0000 @@ -0,0 +1,110 @@ +From: Juergen Gross +Date: Thu, 11 Jun 2020 16:12:41 +0200 +Subject: tools/xenstore: check privilege for XS_IS_DOMAIN_INTRODUCED + +The Xenstore command XS_IS_DOMAIN_INTRODUCED should be possible for +privileged domains only (the only user in the tree is the xenpaging +daemon). + +Instead of having the privilege test for each command introduce a +per-command flag for that purpose. + +This is part of XSA-115. + +Signed-off-by: Juergen Gross +Reviewed-by: Julien Grall +Reviewed-by: Paul Durrant +--- + tools/xenstore/xenstored_core.c | 24 ++++++++++++++++++------ + tools/xenstore/xenstored_domain.c | 7 ++----- + 2 files changed, 20 insertions(+), 11 deletions(-) + +diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c +index 444dea3..b4e6744 100644 +--- a/tools/xenstore/xenstored_core.c ++++ b/tools/xenstore/xenstored_core.c +@@ -1283,8 +1283,10 @@ static struct { + int (*func)(struct connection *conn, struct buffered_data *in); + unsigned int flags; + #define XS_FLAG_NOTID (1U << 0) /* Ignore transaction id. */ ++#define XS_FLAG_PRIV (1U << 1) /* Privileged domain only. */ + } const wire_funcs[XS_TYPE_COUNT] = { +- [XS_CONTROL] = { "CONTROL", do_control }, ++ [XS_CONTROL] = ++ { "CONTROL", do_control, XS_FLAG_PRIV }, + [XS_DIRECTORY] = { "DIRECTORY", send_directory }, + [XS_READ] = { "READ", do_read }, + [XS_GET_PERMS] = { "GET_PERMS", do_get_perms }, +@@ -1294,8 +1296,10 @@ static struct { + { "UNWATCH", do_unwatch, XS_FLAG_NOTID }, + [XS_TRANSACTION_START] = { "TRANSACTION_START", do_transaction_start }, + [XS_TRANSACTION_END] = { "TRANSACTION_END", do_transaction_end }, +- [XS_INTRODUCE] = { "INTRODUCE", do_introduce }, +- [XS_RELEASE] = { "RELEASE", do_release }, ++ [XS_INTRODUCE] = ++ { "INTRODUCE", do_introduce, XS_FLAG_PRIV }, ++ [XS_RELEASE] = ++ { "RELEASE", do_release, XS_FLAG_PRIV }, + [XS_GET_DOMAIN_PATH] = { "GET_DOMAIN_PATH", do_get_domain_path }, + [XS_WRITE] = { "WRITE", do_write }, + [XS_MKDIR] = { "MKDIR", do_mkdir }, +@@ -1304,9 +1308,11 @@ static struct { + [XS_WATCH_EVENT] = { "WATCH_EVENT", NULL }, + [XS_ERROR] = { "ERROR", NULL }, + [XS_IS_DOMAIN_INTRODUCED] = +- { "IS_DOMAIN_INTRODUCED", do_is_domain_introduced }, +- [XS_RESUME] = { "RESUME", do_resume }, +- [XS_SET_TARGET] = { "SET_TARGET", do_set_target }, ++ { "IS_DOMAIN_INTRODUCED", do_is_domain_introduced, XS_FLAG_PRIV }, ++ [XS_RESUME] = ++ { "RESUME", do_resume, XS_FLAG_PRIV }, ++ [XS_SET_TARGET] = ++ { "SET_TARGET", do_set_target, XS_FLAG_PRIV }, + [XS_RESET_WATCHES] = { "RESET_WATCHES", do_reset_watches }, + [XS_DIRECTORY_PART] = { "DIRECTORY_PART", send_directory_part }, + }; +@@ -1334,6 +1340,12 @@ static void process_message(struct connection *conn, struct buffered_data *in) + return; + } + ++ if ((wire_funcs[type].flags & XS_FLAG_PRIV) && ++ domain_is_unprivileged(conn)) { ++ send_error(conn, EACCES); ++ return; ++ } ++ + trans = (wire_funcs[type].flags & XS_FLAG_NOTID) + ? NULL : transaction_lookup(conn, in->hdr.msg.tx_id); + if (IS_ERR(trans)) { +diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c +index eb1f541..e25d2c2 100644 +--- a/tools/xenstore/xenstored_domain.c ++++ b/tools/xenstore/xenstored_domain.c +@@ -385,7 +385,7 @@ int do_introduce(struct connection *conn, struct buffered_data *in) + if (get_strings(in, vec, ARRAY_SIZE(vec)) < ARRAY_SIZE(vec)) + return EINVAL; + +- if (domain_is_unprivileged(conn) || !conn->can_write) ++ if (!conn->can_write) + return EACCES; + + domid = atoi(vec[0]); +@@ -453,7 +453,7 @@ int do_set_target(struct connection *conn, struct buffered_data *in) + if (get_strings(in, vec, ARRAY_SIZE(vec)) < ARRAY_SIZE(vec)) + return EINVAL; + +- if (domain_is_unprivileged(conn) || !conn->can_write) ++ if (!conn->can_write) + return EACCES; + + domid = atoi(vec[0]); +@@ -488,9 +488,6 @@ static struct domain *onearg_domain(struct connection *conn, + if (!domid) + return ERR_PTR(-EINVAL); + +- if (domain_is_unprivileged(conn)) +- return ERR_PTR(-EACCES); +- + return find_connected_domain(domid); + } + diff -Nru xen-4.11.4+57-g41a822c392/debian/patches/0056-tools-xenstore-rework-node-removal.patch xen-4.11.4+57-g41a822c392/debian/patches/0056-tools-xenstore-rework-node-removal.patch --- xen-4.11.4+57-g41a822c392/debian/patches/0056-tools-xenstore-rework-node-removal.patch 1970-01-01 00:00:00.000000000 +0000 +++ xen-4.11.4+57-g41a822c392/debian/patches/0056-tools-xenstore-rework-node-removal.patch 2020-12-11 21:10:09.000000000 +0000 @@ -0,0 +1,213 @@ +From: Juergen Gross +Date: Thu, 11 Jun 2020 16:12:42 +0200 +Subject: tools/xenstore: rework node removal + +Today a Xenstore node is being removed by deleting it from the parent +first and then deleting itself and all its children. This results in +stale entries remaining in the data base in case e.g. a memory +allocation is failing during processing. This would result in the +rather strange behavior to be able to read a node (as its still in the +data base) while not being visible in the tree view of Xenstore. + +Fix that by deleting the nodes from the leaf side instead of starting +at the root. + +As fire_watches() is now called from _rm() the ctx parameter needs a +const attribute. + +This is part of XSA-115. + +Signed-off-by: Juergen Gross +Reviewed-by: Julien Grall +Reviewed-by: Paul Durrant +--- + tools/xenstore/xenstored_core.c | 99 +++++++++++++++++++++------------------- + tools/xenstore/xenstored_watch.c | 4 +- + tools/xenstore/xenstored_watch.h | 2 +- + 3 files changed, 54 insertions(+), 51 deletions(-) + +diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c +index b4e6744..6002cad 100644 +--- a/tools/xenstore/xenstored_core.c ++++ b/tools/xenstore/xenstored_core.c +@@ -1087,74 +1087,76 @@ static int do_mkdir(struct connection *conn, struct buffered_data *in) + return 0; + } + +-static void delete_node(struct connection *conn, struct node *node) +-{ +- unsigned int i; +- char *name; +- +- /* Delete self, then delete children. If we crash, then the worst +- that can happen is the children will continue to take up space, but +- will otherwise be unreachable. */ +- delete_node_single(conn, node); +- +- /* Delete children, too. */ +- for (i = 0; i < node->childlen; i += strlen(node->children+i) + 1) { +- struct node *child; +- +- name = talloc_asprintf(node, "%s/%s", node->name, +- node->children + i); +- child = name ? read_node(conn, node, name) : NULL; +- if (child) { +- delete_node(conn, child); +- } +- else { +- trace("delete_node: Error deleting child '%s/%s'!\n", +- node->name, node->children + i); +- /* Skip it, we've already deleted the parent. */ +- } +- talloc_free(name); +- } +-} +- +- + /* Delete memory using memmove. */ + static void memdel(void *mem, unsigned off, unsigned len, unsigned total) + { + memmove(mem + off, mem + off + len, total - off - len); + } + +- +-static int remove_child_entry(struct connection *conn, struct node *node, +- size_t offset) ++static void remove_child_entry(struct connection *conn, struct node *node, ++ size_t offset) + { + size_t childlen = strlen(node->children + offset); ++ + memdel(node->children, offset, childlen + 1, node->childlen); + node->childlen -= childlen + 1; +- return write_node(conn, node, true); ++ if (write_node(conn, node, true)) ++ corrupt(conn, "Can't update parent node '%s'", node->name); + } + +- +-static int delete_child(struct connection *conn, +- struct node *node, const char *childname) ++static void delete_child(struct connection *conn, ++ struct node *node, const char *childname) + { + unsigned int i; + + for (i = 0; i < node->childlen; i += strlen(node->children+i) + 1) { + if (streq(node->children+i, childname)) { +- return remove_child_entry(conn, node, i); ++ remove_child_entry(conn, node, i); ++ return; + } + } + corrupt(conn, "Can't find child '%s' in %s", childname, node->name); +- return ENOENT; + } + ++static int delete_node(struct connection *conn, struct node *parent, ++ struct node *node) ++{ ++ char *name; ++ ++ /* Delete children. */ ++ while (node->childlen) { ++ struct node *child; ++ ++ name = talloc_asprintf(node, "%s/%s", node->name, ++ node->children); ++ child = name ? read_node(conn, node, name) : NULL; ++ if (child) { ++ if (delete_node(conn, node, child)) ++ return errno; ++ } else { ++ trace("delete_node: Error deleting child '%s/%s'!\n", ++ node->name, node->children); ++ /* Quit deleting. */ ++ errno = ENOMEM; ++ return errno; ++ } ++ talloc_free(name); ++ } ++ ++ delete_node_single(conn, node); ++ delete_child(conn, parent, basename(node->name)); ++ talloc_free(node); ++ ++ return 0; ++} + + static int _rm(struct connection *conn, const void *ctx, struct node *node, + const char *name) + { +- /* Delete from parent first, then if we crash, the worst that can +- happen is the child will continue to take up space, but will +- otherwise be unreachable. */ ++ /* ++ * Deleting node by node, so the result is always consistent even in ++ * case of a failure. ++ */ + struct node *parent; + char *parentname = get_parent(ctx, name); + +@@ -1165,11 +1167,13 @@ static int _rm(struct connection *conn, const void *ctx, struct node *node, + if (!parent) + return (errno == ENOMEM) ? ENOMEM : EINVAL; + +- if (delete_child(conn, parent, basename(name))) +- return EINVAL; +- +- delete_node(conn, node); +- return 0; ++ /* ++ * Fire the watches now, when we can still see the node permissions. ++ * This fine as we are single threaded and the next possible read will ++ * be handled only after the node has been really removed. ++ */ ++ fire_watches(conn, ctx, name, true); ++ return delete_node(conn, parent, node); + } + + +@@ -1207,7 +1211,6 @@ static int do_rm(struct connection *conn, struct buffered_data *in) + if (ret) + return ret; + +- fire_watches(conn, in, name, true); + send_ack(conn, XS_RM); + + return 0; +diff --git a/tools/xenstore/xenstored_watch.c b/tools/xenstore/xenstored_watch.c +index 4580628..cc82864 100644 +--- a/tools/xenstore/xenstored_watch.c ++++ b/tools/xenstore/xenstored_watch.c +@@ -77,7 +77,7 @@ static bool is_child(const char *child, const char *parent) + * Temporary memory allocations are done with ctx. + */ + static void add_event(struct connection *conn, +- void *ctx, ++ const void *ctx, + struct watch *watch, + const char *name) + { +@@ -121,7 +121,7 @@ static void add_event(struct connection *conn, + * Check whether any watch events are to be sent. + * Temporary memory allocations are done with ctx. + */ +-void fire_watches(struct connection *conn, void *ctx, const char *name, ++void fire_watches(struct connection *conn, const void *ctx, const char *name, + bool recurse) + { + struct connection *i; +diff --git a/tools/xenstore/xenstored_watch.h b/tools/xenstore/xenstored_watch.h +index c72ea6a..54d4ea7 100644 +--- a/tools/xenstore/xenstored_watch.h ++++ b/tools/xenstore/xenstored_watch.h +@@ -25,7 +25,7 @@ int do_watch(struct connection *conn, struct buffered_data *in); + int do_unwatch(struct connection *conn, struct buffered_data *in); + + /* Fire all watches: recurse means all the children are affected (ie. rm). */ +-void fire_watches(struct connection *conn, void *tmp, const char *name, ++void fire_watches(struct connection *conn, const void *tmp, const char *name, + bool recurse); + + void conn_delete_all_watches(struct connection *conn); diff -Nru xen-4.11.4+57-g41a822c392/debian/patches/0057-tools-xenstore-fire-watches-only-when-removing-a-spe.patch xen-4.11.4+57-g41a822c392/debian/patches/0057-tools-xenstore-fire-watches-only-when-removing-a-spe.patch --- xen-4.11.4+57-g41a822c392/debian/patches/0057-tools-xenstore-fire-watches-only-when-removing-a-spe.patch 1970-01-01 00:00:00.000000000 +0000 +++ xen-4.11.4+57-g41a822c392/debian/patches/0057-tools-xenstore-fire-watches-only-when-removing-a-spe.patch 2020-12-11 21:10:09.000000000 +0000 @@ -0,0 +1,113 @@ +From: Juergen Gross +Date: Thu, 11 Jun 2020 16:12:43 +0200 +Subject: tools/xenstore: fire watches only when removing a specific node + +Instead of firing all watches for removing a subtree in one go, do so +only when the related node is being removed. + +The watches for the top-most node being removed include all watches +including that node, while watches for nodes below that are only fired +if they are matching exactly. This avoids firing any watch more than +once when removing a subtree. + +This is part of XSA-115. + +Signed-off-by: Juergen Gross +Reviewed-by: Julien Grall +Reviewed-by: Paul Durrant +--- + tools/xenstore/xenstored_core.c | 11 ++++++----- + tools/xenstore/xenstored_watch.c | 13 ++++++++----- + tools/xenstore/xenstored_watch.h | 4 ++-- + 3 files changed, 16 insertions(+), 12 deletions(-) + +diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c +index 6002cad..fcc3798 100644 +--- a/tools/xenstore/xenstored_core.c ++++ b/tools/xenstore/xenstored_core.c +@@ -1118,8 +1118,8 @@ static void delete_child(struct connection *conn, + corrupt(conn, "Can't find child '%s' in %s", childname, node->name); + } + +-static int delete_node(struct connection *conn, struct node *parent, +- struct node *node) ++static int delete_node(struct connection *conn, const void *ctx, ++ struct node *parent, struct node *node) + { + char *name; + +@@ -1131,7 +1131,7 @@ static int delete_node(struct connection *conn, struct node *parent, + node->children); + child = name ? read_node(conn, node, name) : NULL; + if (child) { +- if (delete_node(conn, node, child)) ++ if (delete_node(conn, ctx, node, child)) + return errno; + } else { + trace("delete_node: Error deleting child '%s/%s'!\n", +@@ -1143,6 +1143,7 @@ static int delete_node(struct connection *conn, struct node *parent, + talloc_free(name); + } + ++ fire_watches(conn, ctx, node->name, true); + delete_node_single(conn, node); + delete_child(conn, parent, basename(node->name)); + talloc_free(node); +@@ -1172,8 +1173,8 @@ static int _rm(struct connection *conn, const void *ctx, struct node *node, + * This fine as we are single threaded and the next possible read will + * be handled only after the node has been really removed. + */ +- fire_watches(conn, ctx, name, true); +- return delete_node(conn, parent, node); ++ fire_watches(conn, ctx, name, false); ++ return delete_node(conn, ctx, parent, node); + } + + +diff --git a/tools/xenstore/xenstored_watch.c b/tools/xenstore/xenstored_watch.c +index cc82864..7ca18e0 100644 +--- a/tools/xenstore/xenstored_watch.c ++++ b/tools/xenstore/xenstored_watch.c +@@ -122,7 +122,7 @@ static void add_event(struct connection *conn, + * Temporary memory allocations are done with ctx. + */ + void fire_watches(struct connection *conn, const void *ctx, const char *name, +- bool recurse) ++ bool exact) + { + struct connection *i; + struct watch *watch; +@@ -134,10 +134,13 @@ void fire_watches(struct connection *conn, const void *ctx, const char *name, + /* Create an event for each watch. */ + list_for_each_entry(i, &connections, list) { + list_for_each_entry(watch, &i->watches, list) { +- if (is_child(name, watch->node)) +- add_event(i, ctx, watch, name); +- else if (recurse && is_child(watch->node, name)) +- add_event(i, ctx, watch, watch->node); ++ if (exact) { ++ if (streq(name, watch->node)) ++ add_event(i, ctx, watch, name); ++ } else { ++ if (is_child(name, watch->node)) ++ add_event(i, ctx, watch, name); ++ } + } + } + } +diff --git a/tools/xenstore/xenstored_watch.h b/tools/xenstore/xenstored_watch.h +index 54d4ea7..1b3c80d 100644 +--- a/tools/xenstore/xenstored_watch.h ++++ b/tools/xenstore/xenstored_watch.h +@@ -24,9 +24,9 @@ + int do_watch(struct connection *conn, struct buffered_data *in); + int do_unwatch(struct connection *conn, struct buffered_data *in); + +-/* Fire all watches: recurse means all the children are affected (ie. rm). */ ++/* Fire all watches: !exact means all the children are affected (ie. rm). */ + void fire_watches(struct connection *conn, const void *tmp, const char *name, +- bool recurse); ++ bool exact); + + void conn_delete_all_watches(struct connection *conn); + diff -Nru xen-4.11.4+57-g41a822c392/debian/patches/0058-tools-xenstore-introduce-node_perms-structure.patch xen-4.11.4+57-g41a822c392/debian/patches/0058-tools-xenstore-introduce-node_perms-structure.patch --- xen-4.11.4+57-g41a822c392/debian/patches/0058-tools-xenstore-introduce-node_perms-structure.patch 1970-01-01 00:00:00.000000000 +0000 +++ xen-4.11.4+57-g41a822c392/debian/patches/0058-tools-xenstore-introduce-node_perms-structure.patch 2020-12-11 21:10:09.000000000 +0000 @@ -0,0 +1,285 @@ +From: Juergen Gross +Date: Thu, 11 Jun 2020 16:12:44 +0200 +Subject: tools/xenstore: introduce node_perms structure + +There are several places in xenstored using a permission array and the +size of that array. Introduce a new struct node_perms containing both. + +This is part of XSA-115. + +Signed-off-by: Juergen Gross +Acked-by: Julien Grall +Reviewed-by: Paul Durrant +--- + tools/xenstore/xenstored_core.c | 79 +++++++++++++++++++-------------------- + tools/xenstore/xenstored_core.h | 8 +++- + tools/xenstore/xenstored_domain.c | 12 +++--- + 3 files changed, 50 insertions(+), 49 deletions(-) + +diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c +index fcc3798..f95f44d 100644 +--- a/tools/xenstore/xenstored_core.c ++++ b/tools/xenstore/xenstored_core.c +@@ -401,14 +401,14 @@ static struct node *read_node(struct connection *conn, const void *ctx, + /* Datalen, childlen, number of permissions */ + hdr = (void *)data.dptr; + node->generation = hdr->generation; +- node->num_perms = hdr->num_perms; ++ node->perms.num = hdr->num_perms; + node->datalen = hdr->datalen; + node->childlen = hdr->childlen; + + /* Permissions are struct xs_permissions. */ +- node->perms = hdr->perms; ++ node->perms.p = hdr->perms; + /* Data is binary blob (usually ascii, no nul). */ +- node->data = node->perms + node->num_perms; ++ node->data = node->perms.p + node->perms.num; + /* Children is strings, nul separated. */ + node->children = node->data + node->datalen; + +@@ -425,7 +425,7 @@ int write_node_raw(struct connection *conn, TDB_DATA *key, struct node *node, + struct xs_tdb_record_hdr *hdr; + + data.dsize = sizeof(*hdr) +- + node->num_perms*sizeof(node->perms[0]) ++ + node->perms.num * sizeof(node->perms.p[0]) + + node->datalen + node->childlen; + + if (!no_quota_check && domain_is_unprivileged(conn) && +@@ -437,12 +437,13 @@ int write_node_raw(struct connection *conn, TDB_DATA *key, struct node *node, + data.dptr = talloc_size(node, data.dsize); + hdr = (void *)data.dptr; + hdr->generation = node->generation; +- hdr->num_perms = node->num_perms; ++ hdr->num_perms = node->perms.num; + hdr->datalen = node->datalen; + hdr->childlen = node->childlen; + +- memcpy(hdr->perms, node->perms, node->num_perms*sizeof(node->perms[0])); +- p = hdr->perms + node->num_perms; ++ memcpy(hdr->perms, node->perms.p, ++ node->perms.num * sizeof(*node->perms.p)); ++ p = hdr->perms + node->perms.num; + memcpy(p, node->data, node->datalen); + p += node->datalen; + memcpy(p, node->children, node->childlen); +@@ -468,8 +469,7 @@ static int write_node(struct connection *conn, struct node *node, + } + + static enum xs_perm_type perm_for_conn(struct connection *conn, +- struct xs_permissions *perms, +- unsigned int num) ++ const struct node_perms *perms) + { + unsigned int i; + enum xs_perm_type mask = XS_PERM_READ|XS_PERM_WRITE|XS_PERM_OWNER; +@@ -478,16 +478,16 @@ static enum xs_perm_type perm_for_conn(struct connection *conn, + mask &= ~XS_PERM_WRITE; + + /* Owners and tools get it all... */ +- if (!domain_is_unprivileged(conn) || perms[0].id == conn->id +- || (conn->target && perms[0].id == conn->target->id)) ++ if (!domain_is_unprivileged(conn) || perms->p[0].id == conn->id ++ || (conn->target && perms->p[0].id == conn->target->id)) + return (XS_PERM_READ|XS_PERM_WRITE|XS_PERM_OWNER) & mask; + +- for (i = 1; i < num; i++) +- if (perms[i].id == conn->id +- || (conn->target && perms[i].id == conn->target->id)) +- return perms[i].perms & mask; ++ for (i = 1; i < perms->num; i++) ++ if (perms->p[i].id == conn->id ++ || (conn->target && perms->p[i].id == conn->target->id)) ++ return perms->p[i].perms & mask; + +- return perms[0].perms & mask; ++ return perms->p[0].perms & mask; + } + + /* +@@ -534,7 +534,7 @@ static int ask_parents(struct connection *conn, const void *ctx, + return 0; + } + +- *perm = perm_for_conn(conn, node->perms, node->num_perms); ++ *perm = perm_for_conn(conn, &node->perms); + return 0; + } + +@@ -580,8 +580,7 @@ struct node *get_node(struct connection *conn, + node = read_node(conn, ctx, name); + /* If we don't have permission, we don't have node. */ + if (node) { +- if ((perm_for_conn(conn, node->perms, node->num_perms) & perm) +- != perm) { ++ if ((perm_for_conn(conn, &node->perms) & perm) != perm) { + errno = EACCES; + node = NULL; + } +@@ -757,16 +756,15 @@ const char *onearg(struct buffered_data *in) + return in->buffer; + } + +-static char *perms_to_strings(const void *ctx, +- struct xs_permissions *perms, unsigned int num, ++static char *perms_to_strings(const void *ctx, const struct node_perms *perms, + unsigned int *len) + { + unsigned int i; + char *strings = NULL; + char buffer[MAX_STRLEN(unsigned int) + 1]; + +- for (*len = 0, i = 0; i < num; i++) { +- if (!xs_perm_to_string(&perms[i], buffer, sizeof(buffer))) ++ for (*len = 0, i = 0; i < perms->num; i++) { ++ if (!xs_perm_to_string(&perms->p[i], buffer, sizeof(buffer))) + return NULL; + + strings = talloc_realloc(ctx, strings, char, +@@ -945,13 +943,13 @@ static struct node *construct_node(struct connection *conn, const void *ctx, + goto nomem; + + /* Inherit permissions, except unprivileged domains own what they create */ +- node->num_perms = parent->num_perms; +- node->perms = talloc_memdup(node, parent->perms, +- node->num_perms * sizeof(node->perms[0])); +- if (!node->perms) ++ node->perms.num = parent->perms.num; ++ node->perms.p = talloc_memdup(node, parent->perms.p, ++ node->perms.num * sizeof(*node->perms.p)); ++ if (!node->perms.p) + goto nomem; + if (domain_is_unprivileged(conn)) +- node->perms[0].id = conn->id; ++ node->perms.p[0].id = conn->id; + + /* No children, no data */ + node->children = node->data = NULL; +@@ -1228,7 +1226,7 @@ static int do_get_perms(struct connection *conn, struct buffered_data *in) + if (!node) + return errno; + +- strings = perms_to_strings(node, node->perms, node->num_perms, &len); ++ strings = perms_to_strings(node, &node->perms, &len); + if (!strings) + return errno; + +@@ -1239,13 +1237,12 @@ static int do_get_perms(struct connection *conn, struct buffered_data *in) + + static int do_set_perms(struct connection *conn, struct buffered_data *in) + { +- unsigned int num; +- struct xs_permissions *perms; ++ struct node_perms perms; + char *name, *permstr; + struct node *node; + +- num = xs_count_strings(in->buffer, in->used); +- if (num < 2) ++ perms.num = xs_count_strings(in->buffer, in->used); ++ if (perms.num < 2) + return EINVAL; + + /* First arg is node name. */ +@@ -1256,21 +1253,21 @@ static int do_set_perms(struct connection *conn, struct buffered_data *in) + return errno; + + permstr = in->buffer + strlen(in->buffer) + 1; +- num--; ++ perms.num--; + +- perms = talloc_array(node, struct xs_permissions, num); +- if (!perms) ++ perms.p = talloc_array(node, struct xs_permissions, perms.num); ++ if (!perms.p) + return ENOMEM; +- if (!xs_strings_to_perms(perms, num, permstr)) ++ if (!xs_strings_to_perms(perms.p, perms.num, permstr)) + return errno; + + /* Unprivileged domains may not change the owner. */ +- if (domain_is_unprivileged(conn) && perms[0].id != node->perms[0].id) ++ if (domain_is_unprivileged(conn) && ++ perms.p[0].id != node->perms.p[0].id) + return EPERM; + + domain_entry_dec(conn, node); + node->perms = perms; +- node->num_perms = num; + domain_entry_inc(conn, node); + + if (write_node(conn, node, false)) +@@ -1545,8 +1542,8 @@ static void manual_node(const char *name, const char *child) + barf_perror("Could not allocate initial node %s", name); + + node->name = name; +- node->perms = &perms; +- node->num_perms = 1; ++ node->perms.p = &perms; ++ node->perms.num = 1; + node->children = (char *)child; + if (child) + node->childlen = strlen(child) + 1; +diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h +index ee31237..a43bb55 100644 +--- a/tools/xenstore/xenstored_core.h ++++ b/tools/xenstore/xenstored_core.h +@@ -111,6 +111,11 @@ struct connection + }; + extern struct list_head connections; + ++struct node_perms { ++ unsigned int num; ++ struct xs_permissions *p; ++}; ++ + struct node { + const char *name; + +@@ -122,8 +127,7 @@ struct node { + #define NO_GENERATION ~((uint64_t)0) + + /* Permissions. */ +- unsigned int num_perms; +- struct xs_permissions *perms; ++ struct node_perms perms; + + /* Contents. */ + unsigned int datalen; +diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c +index e25d2c2..433732b 100644 +--- a/tools/xenstore/xenstored_domain.c ++++ b/tools/xenstore/xenstored_domain.c +@@ -665,12 +665,12 @@ void domain_entry_inc(struct connection *conn, struct node *node) + if (!conn) + return; + +- if (node->perms && node->perms[0].id != conn->id) { ++ if (node->perms.p && node->perms.p[0].id != conn->id) { + if (conn->transaction) { + transaction_entry_inc(conn->transaction, +- node->perms[0].id); ++ node->perms.p[0].id); + } else { +- d = find_domain_by_domid(node->perms[0].id); ++ d = find_domain_by_domid(node->perms.p[0].id); + if (d) + d->nbentry++; + } +@@ -691,12 +691,12 @@ void domain_entry_dec(struct connection *conn, struct node *node) + if (!conn) + return; + +- if (node->perms && node->perms[0].id != conn->id) { ++ if (node->perms.p && node->perms.p[0].id != conn->id) { + if (conn->transaction) { + transaction_entry_dec(conn->transaction, +- node->perms[0].id); ++ node->perms.p[0].id); + } else { +- d = find_domain_by_domid(node->perms[0].id); ++ d = find_domain_by_domid(node->perms.p[0].id); + if (d && d->nbentry) + d->nbentry--; + } diff -Nru xen-4.11.4+57-g41a822c392/debian/patches/0059-tools-xenstore-allow-special-watches-for-privileged-.patch xen-4.11.4+57-g41a822c392/debian/patches/0059-tools-xenstore-allow-special-watches-for-privileged-.patch --- xen-4.11.4+57-g41a822c392/debian/patches/0059-tools-xenstore-allow-special-watches-for-privileged-.patch 1970-01-01 00:00:00.000000000 +0000 +++ xen-4.11.4+57-g41a822c392/debian/patches/0059-tools-xenstore-allow-special-watches-for-privileged-.patch 2020-12-11 21:10:09.000000000 +0000 @@ -0,0 +1,232 @@ +From: Juergen Gross +Date: Thu, 11 Jun 2020 16:12:45 +0200 +Subject: tools/xenstore: allow special watches for privileged callers only + +The special watches "@introduceDomain" and "@releaseDomain" should be +allowed for privileged callers only, as they allow to gain information +about presence of other guests on the host. So send watch events for +those watches via privileged connections only. + +In order to allow for disaggregated setups where e.g. driver domains +need to make use of those special watches add support for calling +"set permissions" for those special nodes, too. + +This is part of XSA-115. + +Signed-off-by: Juergen Gross +Reviewed-by: Julien Grall +Reviewed-by: Paul Durrant +--- + docs/misc/xenstore.txt | 5 ++++ + tools/xenstore/xenstored_core.c | 27 +++++++++++------- + tools/xenstore/xenstored_core.h | 2 ++ + tools/xenstore/xenstored_domain.c | 60 +++++++++++++++++++++++++++++++++++++++ + tools/xenstore/xenstored_domain.h | 5 ++++ + tools/xenstore/xenstored_watch.c | 4 +++ + 6 files changed, 93 insertions(+), 10 deletions(-) + +diff --git a/docs/misc/xenstore.txt b/docs/misc/xenstore.txt +index 6f8569d..32969eb 100644 +--- a/docs/misc/xenstore.txt ++++ b/docs/misc/xenstore.txt +@@ -170,6 +170,9 @@ SET_PERMS ||+? + n no access + See http://wiki.xen.org/wiki/XenBus section + `Permissions' for details of the permissions system. ++ It is possible to set permissions for the special watch paths ++ "@introduceDomain" and "@releaseDomain" to enable receiving those ++ watches in unprivileged domains. + + ---------- Watches ---------- + +@@ -194,6 +197,8 @@ WATCH ||? + @releaseDomain occurs on any domain crash or + shutdown, and also on RELEASE + and domain destruction ++ events are sent to privileged callers or explicitly ++ via SET_PERMS enabled domains only. + + When a watch is first set up it is triggered once straight + away, with equal to . Watches may be triggered +diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c +index f95f44d..0308b57 100644 +--- a/tools/xenstore/xenstored_core.c ++++ b/tools/xenstore/xenstored_core.c +@@ -468,8 +468,8 @@ static int write_node(struct connection *conn, struct node *node, + return write_node_raw(conn, &key, node, no_quota_check); + } + +-static enum xs_perm_type perm_for_conn(struct connection *conn, +- const struct node_perms *perms) ++enum xs_perm_type perm_for_conn(struct connection *conn, ++ const struct node_perms *perms) + { + unsigned int i; + enum xs_perm_type mask = XS_PERM_READ|XS_PERM_WRITE|XS_PERM_OWNER; +@@ -1245,22 +1245,29 @@ static int do_set_perms(struct connection *conn, struct buffered_data *in) + if (perms.num < 2) + return EINVAL; + +- /* First arg is node name. */ +- /* We must own node to do this (tools can do this too). */ +- node = get_node_canonicalized(conn, in, in->buffer, &name, +- XS_PERM_WRITE | XS_PERM_OWNER); +- if (!node) +- return errno; +- + permstr = in->buffer + strlen(in->buffer) + 1; + perms.num--; + +- perms.p = talloc_array(node, struct xs_permissions, perms.num); ++ perms.p = talloc_array(in, struct xs_permissions, perms.num); + if (!perms.p) + return ENOMEM; + if (!xs_strings_to_perms(perms.p, perms.num, permstr)) + return errno; + ++ /* First arg is node name. */ ++ if (strstarts(in->buffer, "@")) { ++ if (set_perms_special(conn, in->buffer, &perms)) ++ return errno; ++ send_ack(conn, XS_SET_PERMS); ++ return 0; ++ } ++ ++ /* We must own node to do this (tools can do this too). */ ++ node = get_node_canonicalized(conn, in, in->buffer, &name, ++ XS_PERM_WRITE | XS_PERM_OWNER); ++ if (!node) ++ return errno; ++ + /* Unprivileged domains may not change the owner. */ + if (domain_is_unprivileged(conn) && + perms.p[0].id != node->perms.p[0].id) +diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h +index a43bb55..2092067 100644 +--- a/tools/xenstore/xenstored_core.h ++++ b/tools/xenstore/xenstored_core.h +@@ -167,6 +167,8 @@ struct node *get_node(struct connection *conn, + struct connection *new_connection(connwritefn_t *write, connreadfn_t *read); + void check_store(void); + void corrupt(struct connection *conn, const char *fmt, ...); ++enum xs_perm_type perm_for_conn(struct connection *conn, ++ const struct node_perms *perms); + + /* Is this a valid node name? */ + bool is_valid_nodename(const char *node); +diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c +index 433732b..8888db6 100644 +--- a/tools/xenstore/xenstored_domain.c ++++ b/tools/xenstore/xenstored_domain.c +@@ -41,6 +41,9 @@ static evtchn_port_t virq_port; + + xenevtchn_handle *xce_handle = NULL; + ++static struct node_perms dom_release_perms; ++static struct node_perms dom_introduce_perms; ++ + struct domain + { + struct list_head list; +@@ -597,6 +600,59 @@ void restore_existing_connections(void) + { + } + ++static int set_dom_perms_default(struct node_perms *perms) ++{ ++ perms->num = 1; ++ perms->p = talloc_array(NULL, struct xs_permissions, perms->num); ++ if (!perms->p) ++ return -1; ++ perms->p->id = 0; ++ perms->p->perms = XS_PERM_NONE; ++ ++ return 0; ++} ++ ++static struct node_perms *get_perms_special(const char *name) ++{ ++ if (!strcmp(name, "@releaseDomain")) ++ return &dom_release_perms; ++ if (!strcmp(name, "@introduceDomain")) ++ return &dom_introduce_perms; ++ return NULL; ++} ++ ++int set_perms_special(struct connection *conn, const char *name, ++ struct node_perms *perms) ++{ ++ struct node_perms *p; ++ ++ p = get_perms_special(name); ++ if (!p) ++ return EINVAL; ++ ++ if ((perm_for_conn(conn, p) & (XS_PERM_WRITE | XS_PERM_OWNER)) != ++ (XS_PERM_WRITE | XS_PERM_OWNER)) ++ return EACCES; ++ ++ p->num = perms->num; ++ talloc_free(p->p); ++ p->p = perms->p; ++ talloc_steal(NULL, perms->p); ++ ++ return 0; ++} ++ ++bool check_perms_special(const char *name, struct connection *conn) ++{ ++ struct node_perms *p; ++ ++ p = get_perms_special(name); ++ if (!p) ++ return false; ++ ++ return perm_for_conn(conn, p) & XS_PERM_READ; ++} ++ + static int dom0_init(void) + { + evtchn_port_t port; +@@ -618,6 +674,10 @@ static int dom0_init(void) + + xenevtchn_notify(xce_handle, dom0->port); + ++ if (set_dom_perms_default(&dom_release_perms) || ++ set_dom_perms_default(&dom_introduce_perms)) ++ return -1; ++ + return 0; + } + +diff --git a/tools/xenstore/xenstored_domain.h b/tools/xenstore/xenstored_domain.h +index 56ae015..2591839 100644 +--- a/tools/xenstore/xenstored_domain.h ++++ b/tools/xenstore/xenstored_domain.h +@@ -65,6 +65,11 @@ void domain_watch_inc(struct connection *conn); + void domain_watch_dec(struct connection *conn); + int domain_watch(struct connection *conn); + ++/* Special node permission handling. */ ++int set_perms_special(struct connection *conn, const char *name, ++ struct node_perms *perms); ++bool check_perms_special(const char *name, struct connection *conn); ++ + /* Write rate limiting */ + + #define WRL_FACTOR 1000 /* for fixed-point arithmetic */ +diff --git a/tools/xenstore/xenstored_watch.c b/tools/xenstore/xenstored_watch.c +index 7ca18e0..fc7e5ce 100644 +--- a/tools/xenstore/xenstored_watch.c ++++ b/tools/xenstore/xenstored_watch.c +@@ -133,6 +133,10 @@ void fire_watches(struct connection *conn, const void *ctx, const char *name, + + /* Create an event for each watch. */ + list_for_each_entry(i, &connections, list) { ++ /* introduce/release domain watches */ ++ if (check_special_event(name) && !check_perms_special(name, i)) ++ continue; ++ + list_for_each_entry(watch, &i->watches, list) { + if (exact) { + if (streq(name, watch->node)) diff -Nru xen-4.11.4+57-g41a822c392/debian/patches/0060-tools-xenstore-avoid-watch-events-for-nodes-without-.patch xen-4.11.4+57-g41a822c392/debian/patches/0060-tools-xenstore-avoid-watch-events-for-nodes-without-.patch --- xen-4.11.4+57-g41a822c392/debian/patches/0060-tools-xenstore-avoid-watch-events-for-nodes-without-.patch 1970-01-01 00:00:00.000000000 +0000 +++ xen-4.11.4+57-g41a822c392/debian/patches/0060-tools-xenstore-avoid-watch-events-for-nodes-without-.patch 2020-12-11 21:10:09.000000000 +0000 @@ -0,0 +1,370 @@ +From: Juergen Gross +Date: Thu, 11 Jun 2020 16:12:46 +0200 +Subject: tools/xenstore: avoid watch events for nodes without access + +Today watch events are sent regardless of the access rights of the +node the event is sent for. This enables any guest to e.g. setup a +watch for "/" in order to have a detailed record of all Xenstore +modifications. + +Modify that by sending only watch events for nodes that the watcher +has a chance to see otherwise (either via direct reads or by querying +the children of a node). This includes cases where the visibility of +a node for a watcher is changing (permissions being removed). + +This is part of XSA-115. + +Signed-off-by: Juergen Gross +[julieng: Handle rebase conflict] +Reviewed-by: Julien Grall +Reviewed-by: Paul Durrant +--- + tools/xenstore/xenstored_core.c | 28 +++++++------ + tools/xenstore/xenstored_core.h | 15 ++++--- + tools/xenstore/xenstored_domain.c | 6 +-- + tools/xenstore/xenstored_transaction.c | 21 +++++++++- + tools/xenstore/xenstored_watch.c | 75 +++++++++++++++++++++++++--------- + tools/xenstore/xenstored_watch.h | 2 +- + 6 files changed, 104 insertions(+), 43 deletions(-) + +diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c +index 0308b57..2a86c4a 100644 +--- a/tools/xenstore/xenstored_core.c ++++ b/tools/xenstore/xenstored_core.c +@@ -358,8 +358,8 @@ static void initialize_fds(int sock, int *p_sock_pollfd_idx, + * If it fails, returns NULL and sets errno. + * Temporary memory allocations will be done with ctx. + */ +-static struct node *read_node(struct connection *conn, const void *ctx, +- const char *name) ++struct node *read_node(struct connection *conn, const void *ctx, ++ const char *name) + { + TDB_DATA key, data; + struct xs_tdb_record_hdr *hdr; +@@ -494,7 +494,7 @@ enum xs_perm_type perm_for_conn(struct connection *conn, + * Get name of node parent. + * Temporary memory allocations are done with ctx. + */ +-static char *get_parent(const void *ctx, const char *node) ++char *get_parent(const void *ctx, const char *node) + { + char *parent; + char *slash = strrchr(node + 1, '/'); +@@ -566,10 +566,10 @@ static int errno_from_parents(struct connection *conn, const void *ctx, + * If it fails, returns NULL and sets errno. + * Temporary memory allocations are done with ctx. + */ +-struct node *get_node(struct connection *conn, +- const void *ctx, +- const char *name, +- enum xs_perm_type perm) ++static struct node *get_node(struct connection *conn, ++ const void *ctx, ++ const char *name, ++ enum xs_perm_type perm) + { + struct node *node; + +@@ -1056,7 +1056,7 @@ static int do_write(struct connection *conn, struct buffered_data *in) + return errno; + } + +- fire_watches(conn, in, name, false); ++ fire_watches(conn, in, name, node, false, NULL); + send_ack(conn, XS_WRITE); + + return 0; +@@ -1078,7 +1078,7 @@ static int do_mkdir(struct connection *conn, struct buffered_data *in) + node = create_node(conn, in, name, NULL, 0); + if (!node) + return errno; +- fire_watches(conn, in, name, false); ++ fire_watches(conn, in, name, node, false, NULL); + } + send_ack(conn, XS_MKDIR); + +@@ -1141,7 +1141,7 @@ static int delete_node(struct connection *conn, const void *ctx, + talloc_free(name); + } + +- fire_watches(conn, ctx, node->name, true); ++ fire_watches(conn, ctx, node->name, node, true, NULL); + delete_node_single(conn, node); + delete_child(conn, parent, basename(node->name)); + talloc_free(node); +@@ -1165,13 +1165,14 @@ static int _rm(struct connection *conn, const void *ctx, struct node *node, + parent = read_node(conn, ctx, parentname); + if (!parent) + return (errno == ENOMEM) ? ENOMEM : EINVAL; ++ node->parent = parent; + + /* + * Fire the watches now, when we can still see the node permissions. + * This fine as we are single threaded and the next possible read will + * be handled only after the node has been really removed. + */ +- fire_watches(conn, ctx, name, false); ++ fire_watches(conn, ctx, name, node, false, NULL); + return delete_node(conn, ctx, parent, node); + } + +@@ -1237,7 +1238,7 @@ static int do_get_perms(struct connection *conn, struct buffered_data *in) + + static int do_set_perms(struct connection *conn, struct buffered_data *in) + { +- struct node_perms perms; ++ struct node_perms perms, old_perms; + char *name, *permstr; + struct node *node; + +@@ -1273,6 +1274,7 @@ static int do_set_perms(struct connection *conn, struct buffered_data *in) + perms.p[0].id != node->perms.p[0].id) + return EPERM; + ++ old_perms = node->perms; + domain_entry_dec(conn, node); + node->perms = perms; + domain_entry_inc(conn, node); +@@ -1280,7 +1282,7 @@ static int do_set_perms(struct connection *conn, struct buffered_data *in) + if (write_node(conn, node, false)) + return errno; + +- fire_watches(conn, in, name, false); ++ fire_watches(conn, in, name, node, false, &old_perms); + send_ack(conn, XS_SET_PERMS); + + return 0; +diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h +index 2092067..e4966c8 100644 +--- a/tools/xenstore/xenstored_core.h ++++ b/tools/xenstore/xenstored_core.h +@@ -154,15 +154,17 @@ void send_ack(struct connection *conn, enum xsd_sockmsg_type type); + /* Canonicalize this path if possible. */ + char *canonicalize(struct connection *conn, const void *ctx, const char *node); + ++/* Get access permissions. */ ++enum xs_perm_type perm_for_conn(struct connection *conn, ++ const struct node_perms *perms); ++ + /* Write a node to the tdb data base. */ + int write_node_raw(struct connection *conn, TDB_DATA *key, struct node *node, + bool no_quota_check); + +-/* Get this node, checking we have permissions. */ +-struct node *get_node(struct connection *conn, +- const void *ctx, +- const char *name, +- enum xs_perm_type perm); ++/* Get a node from the tdb data base. */ ++struct node *read_node(struct connection *conn, const void *ctx, ++ const char *name); + + struct connection *new_connection(connwritefn_t *write, connreadfn_t *read); + void check_store(void); +@@ -173,6 +175,9 @@ enum xs_perm_type perm_for_conn(struct connection *conn, + /* Is this a valid node name? */ + bool is_valid_nodename(const char *node); + ++/* Get name of parent node. */ ++char *get_parent(const void *ctx, const char *node); ++ + /* Tracing infrastructure. */ + void trace_create(const void *data, const char *type); + void trace_destroy(const void *data, const char *type); +diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c +index 8888db6..0b2f49a 100644 +--- a/tools/xenstore/xenstored_domain.c ++++ b/tools/xenstore/xenstored_domain.c +@@ -214,7 +214,7 @@ static int destroy_domain(void *_domain) + unmap_interface(domain->interface); + } + +- fire_watches(NULL, domain, "@releaseDomain", false); ++ fire_watches(NULL, domain, "@releaseDomain", NULL, false, NULL); + + wrl_domain_destroy(domain); + +@@ -252,7 +252,7 @@ static void domain_cleanup(void) + } + + if (notify) +- fire_watches(NULL, NULL, "@releaseDomain", false); ++ fire_watches(NULL, NULL, "@releaseDomain", NULL, false, NULL); + } + + /* We scan all domains rather than use the information given here. */ +@@ -418,7 +418,7 @@ int do_introduce(struct connection *conn, struct buffered_data *in) + /* Now domain belongs to its connection. */ + talloc_steal(domain->conn, domain); + +- fire_watches(NULL, in, "@introduceDomain", false); ++ fire_watches(NULL, in, "@introduceDomain", NULL, false, NULL); + } else if ((domain->mfn == mfn) && (domain->conn != conn)) { + /* Use XS_INTRODUCE for recreating the xenbus event-channel. */ + if (domain->port) +diff --git a/tools/xenstore/xenstored_transaction.c b/tools/xenstore/xenstored_transaction.c +index e505429..36793b9 100644 +--- a/tools/xenstore/xenstored_transaction.c ++++ b/tools/xenstore/xenstored_transaction.c +@@ -114,6 +114,9 @@ struct accessed_node + /* Generation count (or NO_GENERATION) for conflict checking. */ + uint64_t generation; + ++ /* Original node permissions. */ ++ struct node_perms perms; ++ + /* Generation count checking required? */ + bool check_gen; + +@@ -260,6 +263,15 @@ int access_node(struct connection *conn, struct node *node, + i->node = talloc_strdup(i, node->name); + if (!i->node) + goto nomem; ++ if (node->generation != NO_GENERATION && node->perms.num) { ++ i->perms.p = talloc_array(i, struct xs_permissions, ++ node->perms.num); ++ if (!i->perms.p) ++ goto nomem; ++ i->perms.num = node->perms.num; ++ memcpy(i->perms.p, node->perms.p, ++ i->perms.num * sizeof(*i->perms.p)); ++ } + + introduce = true; + i->ta_node = false; +@@ -368,9 +380,14 @@ static int finalize_transaction(struct connection *conn, + talloc_free(data.dptr); + if (ret) + goto err; +- } else if (tdb_delete(tdb_ctx, key)) ++ fire_watches(conn, trans, i->node, NULL, false, ++ i->perms.p ? &i->perms : NULL); ++ } else { ++ fire_watches(conn, trans, i->node, NULL, false, ++ i->perms.p ? &i->perms : NULL); ++ if (tdb_delete(tdb_ctx, key)) + goto err; +- fire_watches(conn, trans, i->node, false); ++ } + } + + if (i->ta_node && tdb_delete(tdb_ctx, ta_key)) +diff --git a/tools/xenstore/xenstored_watch.c b/tools/xenstore/xenstored_watch.c +index fc7e5ce..be24797 100644 +--- a/tools/xenstore/xenstored_watch.c ++++ b/tools/xenstore/xenstored_watch.c +@@ -85,22 +85,6 @@ static void add_event(struct connection *conn, + unsigned int len; + char *data; + +- if (!check_special_event(name)) { +- /* Can this conn load node, or see that it doesn't exist? */ +- struct node *node = get_node(conn, ctx, name, XS_PERM_READ); +- /* +- * XXX We allow EACCES here because otherwise a non-dom0 +- * backend driver cannot watch for disappearance of a frontend +- * xenstore directory. When the directory disappears, we +- * revert to permissions of the parent directory for that path, +- * which will typically disallow access for the backend. +- * But this breaks device-channel teardown! +- * Really we should fix this better... +- */ +- if (!node && errno != ENOENT && errno != EACCES) +- return; +- } +- + if (watch->relative_path) { + name += strlen(watch->relative_path); + if (*name == '/') /* Could be "" */ +@@ -117,12 +101,60 @@ static void add_event(struct connection *conn, + talloc_free(data); + } + ++/* ++ * Check permissions of a specific watch to fire: ++ * Either the node itself or its parent have to be readable by the connection ++ * the watch has been setup for. In case a watch event is created due to ++ * changed permissions we need to take the old permissions into account, too. ++ */ ++static bool watch_permitted(struct connection *conn, const void *ctx, ++ const char *name, struct node *node, ++ struct node_perms *perms) ++{ ++ enum xs_perm_type perm; ++ struct node *parent; ++ char *parent_name; ++ ++ if (perms) { ++ perm = perm_for_conn(conn, perms); ++ if (perm & XS_PERM_READ) ++ return true; ++ } ++ ++ if (!node) { ++ node = read_node(conn, ctx, name); ++ if (!node) ++ return false; ++ } ++ ++ perm = perm_for_conn(conn, &node->perms); ++ if (perm & XS_PERM_READ) ++ return true; ++ ++ parent = node->parent; ++ if (!parent) { ++ parent_name = get_parent(ctx, node->name); ++ if (!parent_name) ++ return false; ++ parent = read_node(conn, ctx, parent_name); ++ if (!parent) ++ return false; ++ } ++ ++ perm = perm_for_conn(conn, &parent->perms); ++ ++ return perm & XS_PERM_READ; ++} ++ + /* + * Check whether any watch events are to be sent. + * Temporary memory allocations are done with ctx. ++ * We need to take the (potential) old permissions of the node into account ++ * as a watcher losing permissions to access a node should receive the ++ * watch event, too. + */ + void fire_watches(struct connection *conn, const void *ctx, const char *name, +- bool exact) ++ struct node *node, bool exact, struct node_perms *perms) + { + struct connection *i; + struct watch *watch; +@@ -134,8 +166,13 @@ void fire_watches(struct connection *conn, const void *ctx, const char *name, + /* Create an event for each watch. */ + list_for_each_entry(i, &connections, list) { + /* introduce/release domain watches */ +- if (check_special_event(name) && !check_perms_special(name, i)) +- continue; ++ if (check_special_event(name)) { ++ if (!check_perms_special(name, i)) ++ continue; ++ } else { ++ if (!watch_permitted(i, ctx, name, node, perms)) ++ continue; ++ } + + list_for_each_entry(watch, &i->watches, list) { + if (exact) { +diff --git a/tools/xenstore/xenstored_watch.h b/tools/xenstore/xenstored_watch.h +index 1b3c80d..0309437 100644 +--- a/tools/xenstore/xenstored_watch.h ++++ b/tools/xenstore/xenstored_watch.h +@@ -26,7 +26,7 @@ int do_unwatch(struct connection *conn, struct buffered_data *in); + + /* Fire all watches: !exact means all the children are affected (ie. rm). */ + void fire_watches(struct connection *conn, const void *tmp, const char *name, +- bool exact); ++ struct node *node, bool exact, struct node_perms *perms); + + void conn_delete_all_watches(struct connection *conn); + diff -Nru xen-4.11.4+57-g41a822c392/debian/patches/0061-tools-ocaml-xenstored-ignore-transaction-id-for-un-w.patch xen-4.11.4+57-g41a822c392/debian/patches/0061-tools-ocaml-xenstored-ignore-transaction-id-for-un-w.patch --- xen-4.11.4+57-g41a822c392/debian/patches/0061-tools-ocaml-xenstored-ignore-transaction-id-for-un-w.patch 1970-01-01 00:00:00.000000000 +0000 +++ xen-4.11.4+57-g41a822c392/debian/patches/0061-tools-ocaml-xenstored-ignore-transaction-id-for-un-w.patch 2020-12-11 21:10:09.000000000 +0000 @@ -0,0 +1,47 @@ +From: =?utf-8?b?RWR3aW4gVMO2csO2aw==?= +Date: Fri, 11 Dec 2020 21:50:36 +0100 +Subject: tools/ocaml/xenstored: ignore transaction id for [un]watch +MIME-Version: 1.0 +Content-Type: text/plain; charset="utf-8" +Content-Transfer-Encoding: 8bit + +Instead of ignoring the transaction id for XS_WATCH and XS_UNWATCH +commands as it is documented in docs/misc/xenstore.txt, it is tested +for validity today. + +Really ignore the transaction id for XS_WATCH and XS_UNWATCH. + +This is part of XSA-115. + +Signed-off-by: Edwin Török +Acked-by: Christian Lindig +Reviewed-by: Andrew Cooper +--- + tools/ocaml/xenstored/process.ml | 9 ++++++++- + 1 file changed, 8 insertions(+), 1 deletion(-) + +diff --git a/tools/ocaml/xenstored/process.ml b/tools/ocaml/xenstored/process.ml +index 74c69f8..0a0e43d 100644 +--- a/tools/ocaml/xenstored/process.ml ++++ b/tools/ocaml/xenstored/process.ml +@@ -492,12 +492,19 @@ let retain_op_in_history ty = + | Xenbus.Xb.Op.Reset_watches + | Xenbus.Xb.Op.Invalid -> false + ++let maybe_ignore_transaction = function ++ | Xenbus.Xb.Op.Watch | Xenbus.Xb.Op.Unwatch -> fun tid -> ++ if tid <> Transaction.none then ++ debug "Ignoring transaction ID %d for watch/unwatch" tid; ++ Transaction.none ++ | _ -> fun x -> x ++ + (** + * Nothrow guarantee. + *) + let process_packet ~store ~cons ~doms ~con ~req = + let ty = req.Packet.ty in +- let tid = req.Packet.tid in ++ let tid = maybe_ignore_transaction ty req.Packet.tid in + let rid = req.Packet.rid in + try + let fct = function_of_type ty in diff -Nru xen-4.11.4+57-g41a822c392/debian/patches/0062-tools-ocaml-xenstored-check-privilege-for-XS_IS_DOMA.patch xen-4.11.4+57-g41a822c392/debian/patches/0062-tools-ocaml-xenstored-check-privilege-for-XS_IS_DOMA.patch --- xen-4.11.4+57-g41a822c392/debian/patches/0062-tools-ocaml-xenstored-check-privilege-for-XS_IS_DOMA.patch 1970-01-01 00:00:00.000000000 +0000 +++ xen-4.11.4+57-g41a822c392/debian/patches/0062-tools-ocaml-xenstored-check-privilege-for-XS_IS_DOMA.patch 2020-12-11 21:10:09.000000000 +0000 @@ -0,0 +1,34 @@ +From: =?utf-8?b?RWR3aW4gVMO2csO2aw==?= +Date: Fri, 11 Dec 2020 21:50:36 +0100 +Subject: tools/ocaml/xenstored: check privilege for XS_IS_DOMAIN_INTRODUCED +MIME-Version: 1.0 +Content-Type: text/plain; charset="utf-8" +Content-Transfer-Encoding: 8bit + +The Xenstore command XS_IS_DOMAIN_INTRODUCED should be possible for privileged +domains only (the only user in the tree is the xenpaging daemon). + +This is part of XSA-115. + +Signed-off-by: Edwin Török +Acked-by: Christian Lindig +Reviewed-by: Andrew Cooper +--- + tools/ocaml/xenstored/process.ml | 4 +++- + 1 file changed, 3 insertions(+), 1 deletion(-) + +diff --git a/tools/ocaml/xenstored/process.ml b/tools/ocaml/xenstored/process.ml +index 0a0e43d..f374abe 100644 +--- a/tools/ocaml/xenstored/process.ml ++++ b/tools/ocaml/xenstored/process.ml +@@ -166,7 +166,9 @@ let do_setperms con t domains cons data = + let do_error con t domains cons data = + raise Define.Unknown_operation + +-let do_isintroduced con t domains cons data = ++let do_isintroduced con _t domains _cons data = ++ if not (Connection.is_dom0 con) ++ then raise Define.Permission_denied; + let domid = + match (split None '\000' data) with + | domid :: _ -> int_of_string domid diff -Nru xen-4.11.4+57-g41a822c392/debian/patches/0063-tools-ocaml-xenstored-unify-watch-firing.patch xen-4.11.4+57-g41a822c392/debian/patches/0063-tools-ocaml-xenstored-unify-watch-firing.patch --- xen-4.11.4+57-g41a822c392/debian/patches/0063-tools-ocaml-xenstored-unify-watch-firing.patch 1970-01-01 00:00:00.000000000 +0000 +++ xen-4.11.4+57-g41a822c392/debian/patches/0063-tools-ocaml-xenstored-unify-watch-firing.patch 2020-12-11 21:10:09.000000000 +0000 @@ -0,0 +1,33 @@ +From: =?utf-8?b?RWR3aW4gVMO2csO2aw==?= +Date: Fri, 11 Dec 2020 21:50:36 +0100 +Subject: tools/ocaml/xenstored: unify watch firing +MIME-Version: 1.0 +Content-Type: text/plain; charset="utf-8" +Content-Transfer-Encoding: 8bit + +This will make it easier insert additional checks in a follow-up patch. +All watches are now fired from a single function. + +This is part of XSA-115. + +Signed-off-by: Edwin Török +Acked-by: Christian Lindig +Reviewed-by: Andrew Cooper +--- + tools/ocaml/xenstored/connection.ml | 3 +-- + 1 file changed, 1 insertion(+), 2 deletions(-) + +diff --git a/tools/ocaml/xenstored/connection.ml b/tools/ocaml/xenstored/connection.ml +index be9c62f..d7432c6 100644 +--- a/tools/ocaml/xenstored/connection.ml ++++ b/tools/ocaml/xenstored/connection.ml +@@ -210,8 +210,7 @@ let fire_watch watch path = + end else + path + in +- let data = Utils.join_by_null [ new_path; watch.token; "" ] in +- send_reply watch.con Transaction.none 0 Xenbus.Xb.Op.Watchevent data ++ fire_single_watch { watch with path = new_path } + + (* Search for a valid unused transaction id. *) + let rec valid_transaction_id con proposed_id = diff -Nru xen-4.11.4+57-g41a822c392/debian/patches/0064-tools-ocaml-xenstored-introduce-permissions-for-spec.patch xen-4.11.4+57-g41a822c392/debian/patches/0064-tools-ocaml-xenstored-introduce-permissions-for-spec.patch --- xen-4.11.4+57-g41a822c392/debian/patches/0064-tools-ocaml-xenstored-introduce-permissions-for-spec.patch 1970-01-01 00:00:00.000000000 +0000 +++ xen-4.11.4+57-g41a822c392/debian/patches/0064-tools-ocaml-xenstored-introduce-permissions-for-spec.patch 2020-12-11 21:10:09.000000000 +0000 @@ -0,0 +1,124 @@ +From: =?utf-8?b?RWR3aW4gVMO2csO2aw==?= +Date: Fri, 11 Dec 2020 21:50:36 +0100 +Subject: tools/ocaml/xenstored: introduce permissions for special watches +MIME-Version: 1.0 +Content-Type: text/plain; charset="utf-8" +Content-Transfer-Encoding: 8bit + +The special watches "@introduceDomain" and "@releaseDomain" should be +allowed for privileged callers only, as they allow to gain information +about presence of other guests on the host. So send watch events for +those watches via privileged connections only. + +Start to address this by treating the special watches as regular nodes +in the tree, which gives them normal semantics for permissions. A later +change will restrict the handling, so that they can't be listed, etc. + +This is part of XSA-115. + +Signed-off-by: Edwin Török +Acked-by: Christian Lindig +Reviewed-by: Andrew Cooper +--- + tools/ocaml/xenstored/process.ml | 4 ++-- + tools/ocaml/xenstored/store.ml | 5 +++++ + tools/ocaml/xenstored/utils.ml | 26 ++++++++++++-------------- + tools/ocaml/xenstored/xenstored.ml | 4 +++- + 4 files changed, 22 insertions(+), 17 deletions(-) + +diff --git a/tools/ocaml/xenstored/process.ml b/tools/ocaml/xenstored/process.ml +index f374abe..c3c8ea2 100644 +--- a/tools/ocaml/xenstored/process.ml ++++ b/tools/ocaml/xenstored/process.ml +@@ -414,7 +414,7 @@ let do_introduce con t domains cons data = + else try + let ndom = Domains.create domains domid mfn port in + Connections.add_domain cons ndom; +- Connections.fire_spec_watches cons "@introduceDomain"; ++ Connections.fire_spec_watches cons Store.Path.introduce_domain; + ndom + with _ -> raise Invalid_Cmd_Args + in +@@ -433,7 +433,7 @@ let do_release con t domains cons data = + Domains.del domains domid; + Connections.del_domain cons domid; + if fire_spec_watches +- then Connections.fire_spec_watches cons "@releaseDomain" ++ then Connections.fire_spec_watches cons Store.Path.release_domain + else raise Invalid_Cmd_Args + + let do_resume con t domains cons data = +diff --git a/tools/ocaml/xenstored/store.ml b/tools/ocaml/xenstored/store.ml +index 6375a1c..98d368d 100644 +--- a/tools/ocaml/xenstored/store.ml ++++ b/tools/ocaml/xenstored/store.ml +@@ -214,6 +214,11 @@ let rec lookup node path fct = + + let apply rnode path fct = + lookup rnode path fct ++ ++let introduce_domain = "@introduceDomain" ++let release_domain = "@releaseDomain" ++let specials = List.map of_string [ introduce_domain; release_domain ] ++ + end + + (* The Store.t type *) +diff --git a/tools/ocaml/xenstored/utils.ml b/tools/ocaml/xenstored/utils.ml +index b252db7..e8c9fe4 100644 +--- a/tools/ocaml/xenstored/utils.ml ++++ b/tools/ocaml/xenstored/utils.ml +@@ -88,19 +88,17 @@ let read_file_single_integer filename = + Unix.close fd; + int_of_string (Bytes.sub_string buf 0 sz) + +-let path_complete path connection_path = +- if String.get path 0 <> '/' then +- connection_path ^ path +- else +- path +- ++(* @path may be guest data and needs its length validating. @connection_path ++ * is generated locally in xenstored and always of the form "/local/domain/$N/" *) + let path_validate path connection_path = +- if String.length path = 0 || String.length path > 1024 then +- raise Define.Invalid_path +- else +- let cpath = path_complete path connection_path in +- if String.get cpath 0 <> '/' then +- raise Define.Invalid_path +- else +- cpath ++ let len = String.length path in ++ ++ if len = 0 || len > 1024 then raise Define.Invalid_path; ++ ++ let abs_path = ++ match String.get path 0 with ++ | '/' | '@' -> path ++ | _ -> connection_path ^ path ++ in + ++ abs_path +diff --git a/tools/ocaml/xenstored/xenstored.ml b/tools/ocaml/xenstored/xenstored.ml +index 49fc18b..32c3b1c 100644 +--- a/tools/ocaml/xenstored/xenstored.ml ++++ b/tools/ocaml/xenstored/xenstored.ml +@@ -287,6 +287,8 @@ let _ = + let quit = ref false in + + Logging.init_xenstored_log(); ++ List.iter (fun path -> ++ Store.write store Perms.Connection.full_rights path "") Store.Path.specials; + + let filename = Paths.xen_run_stored ^ "/db" in + if cf.restart && Sys.file_exists filename then ( +@@ -339,7 +341,7 @@ let _ = + let (notify, deaddom) = Domains.cleanup domains in + List.iter (Connections.del_domain cons) deaddom; + if deaddom <> [] || notify then +- Connections.fire_spec_watches cons "@releaseDomain" ++ Connections.fire_spec_watches cons Store.Path.release_domain + ) + else + let c = Connections.find_domain_by_port cons port in diff -Nru xen-4.11.4+57-g41a822c392/debian/patches/0065-tools-ocaml-xenstored-avoid-watch-events-for-nodes-w.patch xen-4.11.4+57-g41a822c392/debian/patches/0065-tools-ocaml-xenstored-avoid-watch-events-for-nodes-w.patch --- xen-4.11.4+57-g41a822c392/debian/patches/0065-tools-ocaml-xenstored-avoid-watch-events-for-nodes-w.patch 1970-01-01 00:00:00.000000000 +0000 +++ xen-4.11.4+57-g41a822c392/debian/patches/0065-tools-ocaml-xenstored-avoid-watch-events-for-nodes-w.patch 2020-12-11 21:10:09.000000000 +0000 @@ -0,0 +1,399 @@ +From: =?utf-8?b?RWR3aW4gVMO2csO2aw==?= +Date: Fri, 11 Dec 2020 21:50:36 +0100 +Subject: tools/ocaml/xenstored: avoid watch events for nodes without access +MIME-Version: 1.0 +Content-Type: text/plain; charset="utf-8" +Content-Transfer-Encoding: 8bit + +Today watch events are sent regardless of the access rights of the +node the event is sent for. This enables any guest to e.g. setup a +watch for "/" in order to have a detailed record of all Xenstore +modifications. + +Modify that by sending only watch events for nodes that the watcher +has a chance to see otherwise (either via direct reads or by querying +the children of a node). This includes cases where the visibility of +a node for a watcher is changing (permissions being removed). + +Permissions for nodes are looked up either in the old (pre +transaction/command) or current trees (post transaction). If +permissions are changed multiple times in a transaction only the final +version is checked, because considering a transaction atomic the +individual permission changes would not be noticable to an outside +observer. + +Two trees are only needed for set_perms: here we can either notice the +node disappearing (if we loose permission), appearing +(if we gain permission), or changing (if we preserve permission). + +RM needs to only look at the old tree: in the new tree the node would be +gone, or could have different permissions if it was recreated (the +recreation would get its own watch fired). + +Inside a tree we lookup the watch path's parent, and then the watch path +child itself. This gets us 4 sets of permissions in worst case, and if +either of these allows a watch, then we permit it to fire. The +permission lookups are done without logging the failures, otherwise we'd +get confusing errors about permission denied for some paths, but a watch +still firing. The actual result is logged in xenstored-access log: + + 'w event ...' as usual if watch was fired + 'w notfired...' if the watch was not fired, together with path and + permission set to help in troubleshooting + +Adding a watch bypasses permission checks and always fires the watch +once immediately. This is consistent with the specification, and no +information is gained (the watch is fired both if the path exists or +doesn't, and both if you have or don't have access, i.e. it reflects the +path a domain gave it back to that domain). + +There are some semantic changes here: + + * Write+rm in a single transaction of the same path is unobservable + now via watches: both before and after a transaction the path + doesn't exist, thus both tree lookups come up with the empty + permission set, and noone, not even Dom0 can see this. This is + consistent with transaction atomicity though. + * Similar to above if we temporarily grant and then revoke permission + on a path any watches fired inbetween are ignored as well + * There is a new log event (w notfired) which shows the permission set + of the path, and the path. + * Watches on paths that a domain doesn't have access to are now not + seen, which is the purpose of the security fix. + +This is part of XSA-115. + +Signed-off-by: Edwin Török +Acked-by: Christian Lindig +Reviewed-by: Andrew Cooper +--- + tools/ocaml/xenstored/connection.ml | 31 ++++++++++++++++++++++++++++--- + tools/ocaml/xenstored/connections.ml | 11 ++++++----- + tools/ocaml/xenstored/logging.ml | 8 ++++++++ + tools/ocaml/xenstored/perms.ml | 18 +++++++++++++----- + tools/ocaml/xenstored/process.ml | 36 ++++++++++++++++++++++-------------- + tools/ocaml/xenstored/transaction.ml | 4 ++++ + tools/ocaml/xenstored/xenstored.ml | 4 +++- + 7 files changed, 84 insertions(+), 28 deletions(-) + +diff --git a/tools/ocaml/xenstored/connection.ml b/tools/ocaml/xenstored/connection.ml +index d7432c6..1389d97 100644 +--- a/tools/ocaml/xenstored/connection.ml ++++ b/tools/ocaml/xenstored/connection.ml +@@ -196,11 +196,36 @@ let list_watches con = + con.watches [] in + List.concat ll + +-let fire_single_watch watch = ++let dbg fmt = Logging.debug "connection" fmt ++let info fmt = Logging.info "connection" fmt ++ ++let lookup_watch_perm path = function ++| None -> [] ++| Some root -> ++ try Store.Path.apply root path @@ fun parent name -> ++ Store.Node.get_perms parent :: ++ try [Store.Node.get_perms (Store.Node.find parent name)] ++ with Not_found -> [] ++ with Define.Invalid_path | Not_found -> [] ++ ++let lookup_watch_perms oldroot root path = ++ lookup_watch_perm path oldroot @ lookup_watch_perm path (Some root) ++ ++let fire_single_watch_unchecked watch = + let data = Utils.join_by_null [watch.path; watch.token; ""] in + send_reply watch.con Transaction.none 0 Xenbus.Xb.Op.Watchevent data + +-let fire_watch watch path = ++let fire_single_watch (oldroot, root) watch = ++ let abspath = get_watch_path watch.con watch.path |> Store.Path.of_string in ++ let perms = lookup_watch_perms oldroot root abspath in ++ if List.exists (Perms.has watch.con.perm READ) perms then ++ fire_single_watch_unchecked watch ++ else ++ let perms = perms |> List.map (Perms.Node.to_string ~sep:" ") |> String.concat ", " in ++ let con = get_domstr watch.con in ++ Logging.watch_not_fired ~con perms (Store.Path.to_string abspath) ++ ++let fire_watch roots watch path = + let new_path = + if watch.is_relative && path.[0] = '/' + then begin +@@ -210,7 +235,7 @@ let fire_watch watch path = + end else + path + in +- fire_single_watch { watch with path = new_path } ++ fire_single_watch roots { watch with path = new_path } + + (* Search for a valid unused transaction id. *) + let rec valid_transaction_id con proposed_id = +diff --git a/tools/ocaml/xenstored/connections.ml b/tools/ocaml/xenstored/connections.ml +index ae76928..020b875 100644 +--- a/tools/ocaml/xenstored/connections.ml ++++ b/tools/ocaml/xenstored/connections.ml +@@ -135,25 +135,26 @@ let del_watch cons con path token = + watch + + (* path is absolute *) +-let fire_watches cons path recurse = ++let fire_watches ?oldroot root cons path recurse = + let key = key_of_path path in + let path = Store.Path.to_string path in ++ let roots = oldroot, root in + let fire_watch _ = function + | None -> () +- | Some watches -> List.iter (fun w -> Connection.fire_watch w path) watches ++ | Some watches -> List.iter (fun w -> Connection.fire_watch roots w path) watches + in + let fire_rec x = function + | None -> () + | Some watches -> +- List.iter (fun w -> Connection.fire_single_watch w) watches ++ List.iter (Connection.fire_single_watch roots) watches + in + Trie.iter_path fire_watch cons.watches key; + if recurse then + Trie.iter fire_rec (Trie.sub cons.watches key) + +-let fire_spec_watches cons specpath = ++let fire_spec_watches root cons specpath = + iter cons (fun con -> +- List.iter (fun w -> Connection.fire_single_watch w) (Connection.get_watches con specpath)) ++ List.iter (Connection.fire_single_watch (None, root)) (Connection.get_watches con specpath)) + + let set_target cons domain target_domain = + let con = find_domain cons domain in +diff --git a/tools/ocaml/xenstored/logging.ml b/tools/ocaml/xenstored/logging.ml +index ea60331..99c7bc5 100644 +--- a/tools/ocaml/xenstored/logging.ml ++++ b/tools/ocaml/xenstored/logging.ml +@@ -161,6 +161,8 @@ let xenstored_log_nb_lines = ref 13215 + let xenstored_log_nb_chars = ref (-1) + let xenstored_logger = ref (None: logger option) + ++let debug_enabled () = !xenstored_log_level = Debug ++ + let set_xenstored_log_destination s = + xenstored_log_destination := log_destination_of_string s + +@@ -204,6 +206,7 @@ type access_type = + | Commit + | Newconn + | Endconn ++ | Watch_not_fired + | XbOp of Xenbus.Xb.Op.operation + + let string_of_tid ~con tid = +@@ -217,6 +220,7 @@ let string_of_access_type = function + | Commit -> "commit " + | Newconn -> "newconn " + | Endconn -> "endconn " ++ | Watch_not_fired -> "w notfired" + + | XbOp op -> match op with + | Xenbus.Xb.Op.Debug -> "debug " +@@ -331,3 +335,7 @@ let xb_answer ~tid ~con ~ty data = + | _ -> false, Debug + in + if print then access_logging ~tid ~con ~data (XbOp ty) ~level ++ ++let watch_not_fired ~con perms path = ++ let data = Printf.sprintf "EPERM perms=[%s] path=%s" perms path in ++ access_logging ~tid:0 ~con ~data Watch_not_fired ~level:Info +diff --git a/tools/ocaml/xenstored/perms.ml b/tools/ocaml/xenstored/perms.ml +index 3ea193e..23b80ab 100644 +--- a/tools/ocaml/xenstored/perms.ml ++++ b/tools/ocaml/xenstored/perms.ml +@@ -79,9 +79,9 @@ let of_string s = + let string_of_perm perm = + Printf.sprintf "%c%u" (char_of_permty (snd perm)) (fst perm) + +-let to_string permvec = ++let to_string ?(sep="\000") permvec = + let l = ((permvec.owner, permvec.other) :: permvec.acl) in +- String.concat "\000" (List.map string_of_perm l) ++ String.concat sep (List.map string_of_perm l) + + end + +@@ -132,8 +132,8 @@ let check_owner (connection:Connection.t) (node:Node.t) = + then Connection.is_owner connection (Node.get_owner node) + else true + +-(* check if the current connection has the requested perm on the current node *) +-let check (connection:Connection.t) request (node:Node.t) = ++(* check if the current connection lacks the requested perm on the current node *) ++let lacks (connection:Connection.t) request (node:Node.t) = + let check_acl domainid = + let perm = + if List.mem_assoc domainid (Node.get_acl node) +@@ -154,11 +154,19 @@ let check (connection:Connection.t) request (node:Node.t) = + info "Permission denied: Domain %d has write only access" domainid; + false + in +- if !activate ++ !activate + && not (Connection.is_dom0 connection) + && not (check_owner connection node) + && not (List.exists check_acl (Connection.get_owners connection)) ++ ++(* check if the current connection has the requested perm on the current node. ++* Raises an exception if it doesn't. *) ++let check connection request node = ++ if lacks connection request node + then raise Define.Permission_denied + ++(* check if the current connection has the requested perm on the current node *) ++let has connection request node = not (lacks connection request node) ++ + let equiv perm1 perm2 = + (Node.to_string perm1) = (Node.to_string perm2) +diff --git a/tools/ocaml/xenstored/process.ml b/tools/ocaml/xenstored/process.ml +index c3c8ea2..3cd0097 100644 +--- a/tools/ocaml/xenstored/process.ml ++++ b/tools/ocaml/xenstored/process.ml +@@ -56,15 +56,17 @@ let split_one_path data con = + | path :: "" :: [] -> Store.Path.create path (Connection.get_path con) + | _ -> raise Invalid_Cmd_Args + +-let process_watch ops cons = ++let process_watch t cons = ++ let oldroot = t.Transaction.oldroot in ++ let newroot = Store.get_root t.store in ++ let ops = Transaction.get_paths t |> List.rev in + let do_op_watch op cons = +- let recurse = match (fst op) with +- | Xenbus.Xb.Op.Write -> false +- | Xenbus.Xb.Op.Mkdir -> false +- | Xenbus.Xb.Op.Rm -> true +- | Xenbus.Xb.Op.Setperms -> false ++ let recurse, oldroot, root = match (fst op) with ++ | Xenbus.Xb.Op.Write|Xenbus.Xb.Op.Mkdir -> false, None, newroot ++ | Xenbus.Xb.Op.Rm -> true, None, oldroot ++ | Xenbus.Xb.Op.Setperms -> false, Some oldroot, newroot + | _ -> raise (Failure "huh ?") in +- Connections.fire_watches cons (snd op) recurse in ++ Connections.fire_watches ?oldroot root cons (snd op) recurse in + List.iter (fun op -> do_op_watch op cons) ops + + let create_implicit_path t perm path = +@@ -205,7 +207,7 @@ let reply_ack fct con t doms cons data = + fct con t doms cons data; + Packet.Ack (fun () -> + if Transaction.get_id t = Transaction.none then +- process_watch (Transaction.get_paths t) cons ++ process_watch t cons + ) + + let reply_data fct con t doms cons data = +@@ -353,14 +355,17 @@ let transaction_replay c t doms cons = + Connection.end_transaction c tid None + ) + +-let do_watch con t domains cons data = ++let do_watch con t _domains cons data = + let (node, token) = + match (split None '\000' data) with + | [node; token; ""] -> node, token + | _ -> raise Invalid_Cmd_Args + in + let watch = Connections.add_watch cons con node token in +- Packet.Ack (fun () -> Connection.fire_single_watch watch) ++ Packet.Ack (fun () -> ++ (* xenstore.txt says this watch is fired immediately, ++ implying even if path doesn't exist or is unreadable *) ++ Connection.fire_single_watch_unchecked watch) + + let do_unwatch con t domains cons data = + let (node, token) = +@@ -391,7 +396,7 @@ let do_transaction_end con t domains cons data = + if not success then + raise Transaction_again; + if commit then begin +- process_watch (List.rev (Transaction.get_paths t)) cons; ++ process_watch t cons; + match t.Transaction.ty with + | Transaction.No -> + () (* no need to record anything *) +@@ -414,7 +419,7 @@ let do_introduce con t domains cons data = + else try + let ndom = Domains.create domains domid mfn port in + Connections.add_domain cons ndom; +- Connections.fire_spec_watches cons Store.Path.introduce_domain; ++ Connections.fire_spec_watches (Transaction.get_root t) cons Store.Path.introduce_domain; + ndom + with _ -> raise Invalid_Cmd_Args + in +@@ -433,7 +438,7 @@ let do_release con t domains cons data = + Domains.del domains domid; + Connections.del_domain cons domid; + if fire_spec_watches +- then Connections.fire_spec_watches cons Store.Path.release_domain ++ then Connections.fire_spec_watches (Transaction.get_root t) cons Store.Path.release_domain + else raise Invalid_Cmd_Args + + let do_resume con t domains cons data = +@@ -501,6 +506,8 @@ let maybe_ignore_transaction = function + Transaction.none + | _ -> fun x -> x + ++ ++let () = Printexc.record_backtrace true + (** + * Nothrow guarantee. + *) +@@ -542,7 +549,8 @@ let process_packet ~store ~cons ~doms ~con ~req = + (* Put the response on the wire *) + send_response ty con t rid response + with exn -> +- error "process packet: %s" (Printexc.to_string exn); ++ let bt = Printexc.get_backtrace () in ++ error "process packet: %s. %s" (Printexc.to_string exn) bt; + Connection.send_error con tid rid "EIO" + + let do_input store cons doms con = +diff --git a/tools/ocaml/xenstored/transaction.ml b/tools/ocaml/xenstored/transaction.ml +index 23e7ccf..9e9e28d 100644 +--- a/tools/ocaml/xenstored/transaction.ml ++++ b/tools/ocaml/xenstored/transaction.ml +@@ -82,6 +82,7 @@ type t = { + start_count: int64; + store: Store.t; (* This is the store that we change in write operations. *) + quota: Quota.t; ++ oldroot: Store.Node.t; + mutable paths: (Xenbus.Xb.Op.operation * Store.Path.t) list; + mutable operations: (Packet.request * Packet.response) list; + mutable read_lowpath: Store.Path.t option; +@@ -123,6 +124,7 @@ let make ?(internal=false) id store = + start_count = !counter; + store = if id = none then store else Store.copy store; + quota = Quota.copy store.Store.quota; ++ oldroot = Store.get_root store; + paths = []; + operations = []; + read_lowpath = None; +@@ -137,6 +139,8 @@ let make ?(internal=false) id store = + let get_store t = t.store + let get_paths t = t.paths + ++let get_root t = Store.get_root t.store ++ + let is_read_only t = t.paths = [] + let add_wop t ty path = t.paths <- (ty, path) :: t.paths + let add_operation ~perm t request response = +diff --git a/tools/ocaml/xenstored/xenstored.ml b/tools/ocaml/xenstored/xenstored.ml +index 32c3b1c..e9f4718 100644 +--- a/tools/ocaml/xenstored/xenstored.ml ++++ b/tools/ocaml/xenstored/xenstored.ml +@@ -341,7 +341,9 @@ let _ = + let (notify, deaddom) = Domains.cleanup domains in + List.iter (Connections.del_domain cons) deaddom; + if deaddom <> [] || notify then +- Connections.fire_spec_watches cons Store.Path.release_domain ++ Connections.fire_spec_watches ++ (Store.get_root store) ++ cons Store.Path.release_domain + ) + else + let c = Connections.find_domain_by_port cons port in diff -Nru xen-4.11.4+57-g41a822c392/debian/patches/0066-tools-ocaml-xenstored-add-xenstored.conf-flag-to-tur.patch xen-4.11.4+57-g41a822c392/debian/patches/0066-tools-ocaml-xenstored-add-xenstored.conf-flag-to-tur.patch --- xen-4.11.4+57-g41a822c392/debian/patches/0066-tools-ocaml-xenstored-add-xenstored.conf-flag-to-tur.patch 1970-01-01 00:00:00.000000000 +0000 +++ xen-4.11.4+57-g41a822c392/debian/patches/0066-tools-ocaml-xenstored-add-xenstored.conf-flag-to-tur.patch 2020-12-11 21:10:09.000000000 +0000 @@ -0,0 +1,91 @@ +From: =?utf-8?b?RWR3aW4gVMO2csO2aw==?= +Date: Fri, 11 Dec 2020 21:50:36 +0100 +Subject: tools/ocaml/xenstored: add xenstored.conf flag to turn off watch + permission checks +MIME-Version: 1.0 +Content-Type: text/plain; charset="utf-8" +Content-Transfer-Encoding: 8bit + +There are flags to turn off quotas and the permission system, so add one +that turns off the newly introduced watch permission checks as well. + +This is part of XSA-115. + +Signed-off-by: Edwin Török +Acked-by: Christian Lindig +Reviewed-by: Andrew Cooper +--- + tools/ocaml/xenstored/connection.ml | 2 +- + tools/ocaml/xenstored/oxenstored.conf.in | 10 ++++++++++ + tools/ocaml/xenstored/perms.ml | 5 +++++ + tools/ocaml/xenstored/xenstored.ml | 1 + + 4 files changed, 17 insertions(+), 1 deletion(-) + +diff --git a/tools/ocaml/xenstored/connection.ml b/tools/ocaml/xenstored/connection.ml +index 1389d97..698f721 100644 +--- a/tools/ocaml/xenstored/connection.ml ++++ b/tools/ocaml/xenstored/connection.ml +@@ -218,7 +218,7 @@ let fire_single_watch_unchecked watch = + let fire_single_watch (oldroot, root) watch = + let abspath = get_watch_path watch.con watch.path |> Store.Path.of_string in + let perms = lookup_watch_perms oldroot root abspath in +- if List.exists (Perms.has watch.con.perm READ) perms then ++ if Perms.can_fire_watch watch.con.perm perms then + fire_single_watch_unchecked watch + else + let perms = perms |> List.map (Perms.Node.to_string ~sep:" ") |> String.concat ", " in +diff --git a/tools/ocaml/xenstored/oxenstored.conf.in b/tools/ocaml/xenstored/oxenstored.conf.in +index 6579b84..d5d4f00 100644 +--- a/tools/ocaml/xenstored/oxenstored.conf.in ++++ b/tools/ocaml/xenstored/oxenstored.conf.in +@@ -44,6 +44,16 @@ conflict-rate-limit-is-aggregate = true + # Activate node permission system + perms-activate = true + ++# Activate the watch permission system ++# When this is enabled unprivileged guests can only get watch events ++# for xenstore entries that they would've been able to read. ++# ++# When this is disabled unprivileged guests may get watch events ++# for xenstore entries that they cannot read. The watch event contains ++# only the entry name, not the value. ++# This restores behaviour prior to XSA-115. ++perms-watch-activate = true ++ + # Activate quota + quota-activate = true + quota-maxentity = 1000 +diff --git a/tools/ocaml/xenstored/perms.ml b/tools/ocaml/xenstored/perms.ml +index 23b80ab..ee7fee6 100644 +--- a/tools/ocaml/xenstored/perms.ml ++++ b/tools/ocaml/xenstored/perms.ml +@@ -20,6 +20,7 @@ let info fmt = Logging.info "perms" fmt + open Stdext + + let activate = ref true ++let watch_activate = ref true + + type permty = READ | WRITE | RDWR | NONE + +@@ -168,5 +169,9 @@ let check connection request node = + (* check if the current connection has the requested perm on the current node *) + let has connection request node = not (lacks connection request node) + ++let can_fire_watch connection perms = ++ not !watch_activate ++ || List.exists (has connection READ) perms ++ + let equiv perm1 perm2 = + (Node.to_string perm1) = (Node.to_string perm2) +diff --git a/tools/ocaml/xenstored/xenstored.ml b/tools/ocaml/xenstored/xenstored.ml +index e9f4718..30fc874 100644 +--- a/tools/ocaml/xenstored/xenstored.ml ++++ b/tools/ocaml/xenstored/xenstored.ml +@@ -95,6 +95,7 @@ let parse_config filename = + ("conflict-max-history-seconds", Config.Set_float Define.conflict_max_history_seconds); + ("conflict-rate-limit-is-aggregate", Config.Set_bool Define.conflict_rate_limit_is_aggregate); + ("perms-activate", Config.Set_bool Perms.activate); ++ ("perms-watch-activate", Config.Set_bool Perms.watch_activate); + ("quota-activate", Config.Set_bool Quota.activate); + ("quota-maxwatch", Config.Set_int Define.maxwatch); + ("quota-transaction", Config.Set_int Define.maxtransaction); diff -Nru xen-4.11.4+57-g41a822c392/debian/patches/0067-tools-xenstore-revoke-access-rights-for-removed-doma.patch xen-4.11.4+57-g41a822c392/debian/patches/0067-tools-xenstore-revoke-access-rights-for-removed-doma.patch --- xen-4.11.4+57-g41a822c392/debian/patches/0067-tools-xenstore-revoke-access-rights-for-removed-doma.patch 1970-01-01 00:00:00.000000000 +0000 +++ xen-4.11.4+57-g41a822c392/debian/patches/0067-tools-xenstore-revoke-access-rights-for-removed-doma.patch 2020-12-11 21:10:09.000000000 +0000 @@ -0,0 +1,542 @@ +From: Juergen Gross +Date: Fri, 11 Dec 2020 21:51:24 +0100 +Subject: tools/xenstore: revoke access rights for removed domains + +Access rights of Xenstore nodes are per domid. Unfortunately existing +granted access rights are not removed when a domain is being destroyed. +This means that a new domain created with the same domid will inherit +the access rights to Xenstore nodes from the previous domain(s) with +the same domid. + +This can be avoided by adding a generation counter to each domain. +The generation counter of the domain is set to the global generation +counter when a domain structure is being allocated. When reading or +writing a node all permissions of domains which are younger than the +node itself are dropped. This is done by flagging the related entry +as invalid in order to avoid modifying permissions in a way the user +could detect. + +A special case has to be considered: for a new domain the first +Xenstore entries are already written before the domain is officially +introduced in Xenstore. In order not to drop the permissions for the +new domain a domain struct is allocated even before introduction if +the hypervisor is aware of the domain. This requires adding another +bool "introduced" to struct domain in xenstored. In order to avoid +additional padding holes convert the shutdown flag to bool, too. + +As verifying permissions has its price regarding runtime add a new +quota for limiting the number of permissions an unprivileged domain +can set for a node. The default for that new quota is 5. + +This is part of XSA-322. + +Signed-off-by: Juergen Gross +Reviewed-by: Paul Durrant +Acked-by: Julien Grall +--- + tools/xenstore/include/xenstore_lib.h | 1 + + tools/xenstore/xenstored_core.c | 29 ++++- + tools/xenstore/xenstored_domain.c | 186 ++++++++++++++++++++++++++------- + tools/xenstore/xenstored_domain.h | 3 + + tools/xenstore/xenstored_transaction.c | 15 ++- + tools/xenstore/xenstored_transaction.h | 2 + + tools/xenstore/xs_lib.c | 2 +- + 7 files changed, 192 insertions(+), 46 deletions(-) + +diff --git a/tools/xenstore/include/xenstore_lib.h b/tools/xenstore/include/xenstore_lib.h +index 0ffbae9..4c9b6d1 100644 +--- a/tools/xenstore/include/xenstore_lib.h ++++ b/tools/xenstore/include/xenstore_lib.h +@@ -34,6 +34,7 @@ enum xs_perm_type { + /* Internal use. */ + XS_PERM_ENOENT_OK = 4, + XS_PERM_OWNER = 8, ++ XS_PERM_IGNORE = 16, + }; + + struct xs_permissions +diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c +index 2a86c4a..4fbe5c7 100644 +--- a/tools/xenstore/xenstored_core.c ++++ b/tools/xenstore/xenstored_core.c +@@ -101,6 +101,7 @@ int quota_nb_entry_per_domain = 1000; + int quota_nb_watch_per_domain = 128; + int quota_max_entry_size = 2048; /* 2K */ + int quota_max_transaction = 10; ++int quota_nb_perms_per_node = 5; + + void trace(const char *fmt, ...) + { +@@ -407,8 +408,13 @@ struct node *read_node(struct connection *conn, const void *ctx, + + /* Permissions are struct xs_permissions. */ + node->perms.p = hdr->perms; ++ if (domain_adjust_node_perms(node)) { ++ talloc_free(node); ++ return NULL; ++ } ++ + /* Data is binary blob (usually ascii, no nul). */ +- node->data = node->perms.p + node->perms.num; ++ node->data = node->perms.p + hdr->num_perms; + /* Children is strings, nul separated. */ + node->children = node->data + node->datalen; + +@@ -424,6 +430,9 @@ int write_node_raw(struct connection *conn, TDB_DATA *key, struct node *node, + void *p; + struct xs_tdb_record_hdr *hdr; + ++ if (domain_adjust_node_perms(node)) ++ return errno; ++ + data.dsize = sizeof(*hdr) + + node->perms.num * sizeof(node->perms.p[0]) + + node->datalen + node->childlen; +@@ -483,8 +492,9 @@ enum xs_perm_type perm_for_conn(struct connection *conn, + return (XS_PERM_READ|XS_PERM_WRITE|XS_PERM_OWNER) & mask; + + for (i = 1; i < perms->num; i++) +- if (perms->p[i].id == conn->id +- || (conn->target && perms->p[i].id == conn->target->id)) ++ if (!(perms->p[i].perms & XS_PERM_IGNORE) && ++ (perms->p[i].id == conn->id || ++ (conn->target && perms->p[i].id == conn->target->id))) + return perms->p[i].perms & mask; + + return perms->p[0].perms & mask; +@@ -1246,8 +1256,12 @@ static int do_set_perms(struct connection *conn, struct buffered_data *in) + if (perms.num < 2) + return EINVAL; + +- permstr = in->buffer + strlen(in->buffer) + 1; + perms.num--; ++ if (domain_is_unprivileged(conn) && ++ perms.num > quota_nb_perms_per_node) ++ return ENOSPC; ++ ++ permstr = in->buffer + strlen(in->buffer) + 1; + + perms.p = talloc_array(in, struct xs_permissions, perms.num); + if (!perms.p) +@@ -1919,6 +1933,7 @@ static void usage(void) + " -S, --entry-size limit the size of entry per domain, and\n" + " -W, --watch-nb limit the number of watches per domain,\n" + " -t, --transaction limit the number of transaction allowed per domain,\n" ++" -A, --perm-nb limit the number of permissions per node,\n" + " -R, --no-recovery to request that no recovery should be attempted when\n" + " the store is corrupted (debug only),\n" + " -I, --internal-db store database in memory, not on disk\n" +@@ -1939,6 +1954,7 @@ static struct option options[] = { + { "entry-size", 1, NULL, 'S' }, + { "trace-file", 1, NULL, 'T' }, + { "transaction", 1, NULL, 't' }, ++ { "perm-nb", 1, NULL, 'A' }, + { "no-recovery", 0, NULL, 'R' }, + { "internal-db", 0, NULL, 'I' }, + { "verbose", 0, NULL, 'V' }, +@@ -1961,7 +1977,7 @@ int main(int argc, char *argv[]) + int timeout; + + +- while ((opt = getopt_long(argc, argv, "DE:F:HNPS:t:T:RVW:", options, ++ while ((opt = getopt_long(argc, argv, "DE:F:HNPS:t:A:T:RVW:", options, + NULL)) != -1) { + switch (opt) { + case 'D': +@@ -2003,6 +2019,9 @@ int main(int argc, char *argv[]) + case 'W': + quota_nb_watch_per_domain = strtol(optarg, NULL, 10); + break; ++ case 'A': ++ quota_nb_perms_per_node = strtol(optarg, NULL, 10); ++ break; + case 'e': + dom0_event = strtol(optarg, NULL, 10); + break; +diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c +index 0b2f49a..f5e7af4 100644 +--- a/tools/xenstore/xenstored_domain.c ++++ b/tools/xenstore/xenstored_domain.c +@@ -71,8 +71,14 @@ struct domain + /* The connection associated with this. */ + struct connection *conn; + ++ /* Generation count at domain introduction time. */ ++ uint64_t generation; ++ + /* Have we noticed that this domain is shutdown? */ +- int shutdown; ++ bool shutdown; ++ ++ /* Has domain been officially introduced? */ ++ bool introduced; + + /* number of entry from this domain in the store */ + int nbentry; +@@ -200,6 +206,9 @@ static int destroy_domain(void *_domain) + + list_del(&domain->list); + ++ if (!domain->introduced) ++ return 0; ++ + if (domain->port) { + if (xenevtchn_unbind(xce_handle, domain->port) == -1) + eprintf("> Unbinding port %i failed!\n", domain->port); +@@ -221,21 +230,34 @@ static int destroy_domain(void *_domain) + return 0; + } + ++static bool get_domain_info(unsigned int domid, xc_dominfo_t *dominfo) ++{ ++ return xc_domain_getinfo(*xc_handle, domid, 1, dominfo) == 1 && ++ dominfo->domid == domid; ++} ++ + static void domain_cleanup(void) + { + xc_dominfo_t dominfo; + struct domain *domain; + struct connection *conn; + int notify = 0; ++ bool dom_valid; + + again: + list_for_each_entry(domain, &domains, list) { +- if (xc_domain_getinfo(*xc_handle, domain->domid, 1, +- &dominfo) == 1 && +- dominfo.domid == domain->domid) { ++ dom_valid = get_domain_info(domain->domid, &dominfo); ++ if (!domain->introduced) { ++ if (!dom_valid) { ++ talloc_free(domain); ++ goto again; ++ } ++ continue; ++ } ++ if (dom_valid) { + if ((dominfo.crashed || dominfo.shutdown) + && !domain->shutdown) { +- domain->shutdown = 1; ++ domain->shutdown = true; + notify = 1; + } + if (!dominfo.dying) +@@ -301,58 +323,84 @@ static char *talloc_domain_path(void *context, unsigned int domid) + return talloc_asprintf(context, "/local/domain/%u", domid); + } + +-static struct domain *new_domain(void *context, unsigned int domid, +- int port) ++static struct domain *find_domain_struct(unsigned int domid) ++{ ++ struct domain *i; ++ ++ list_for_each_entry(i, &domains, list) { ++ if (i->domid == domid) ++ return i; ++ } ++ return NULL; ++} ++ ++static struct domain *alloc_domain(void *context, unsigned int domid) + { + struct domain *domain; +- int rc; + + domain = talloc(context, struct domain); +- if (!domain) ++ if (!domain) { ++ errno = ENOMEM; + return NULL; ++ } + +- domain->port = 0; +- domain->shutdown = 0; + domain->domid = domid; +- domain->path = talloc_domain_path(domain, domid); +- if (!domain->path) +- return NULL; ++ domain->generation = generation; ++ domain->introduced = false; + +- wrl_domain_new(domain); ++ talloc_set_destructor(domain, destroy_domain); + + list_add(&domain->list, &domains); +- talloc_set_destructor(domain, destroy_domain); ++ ++ return domain; ++} ++ ++static int new_domain(struct domain *domain, int port) ++{ ++ int rc; ++ ++ domain->port = 0; ++ domain->shutdown = false; ++ domain->path = talloc_domain_path(domain, domain->domid); ++ if (!domain->path) { ++ errno = ENOMEM; ++ return errno; ++ } ++ ++ wrl_domain_new(domain); + + /* Tell kernel we're interested in this event. */ +- rc = xenevtchn_bind_interdomain(xce_handle, domid, port); ++ rc = xenevtchn_bind_interdomain(xce_handle, domain->domid, port); + if (rc == -1) +- return NULL; ++ return errno; + domain->port = rc; + ++ domain->introduced = true; ++ + domain->conn = new_connection(writechn, readchn); +- if (!domain->conn) +- return NULL; ++ if (!domain->conn) { ++ errno = ENOMEM; ++ return errno; ++ } + + domain->conn->domain = domain; +- domain->conn->id = domid; ++ domain->conn->id = domain->domid; + + domain->remote_port = port; + domain->nbentry = 0; + domain->nbwatch = 0; + +- return domain; ++ return 0; + } + + + static struct domain *find_domain_by_domid(unsigned int domid) + { +- struct domain *i; ++ struct domain *d; + +- list_for_each_entry(i, &domains, list) { +- if (i->domid == domid) +- return i; +- } +- return NULL; ++ d = find_domain_struct(domid); ++ ++ return (d && d->introduced) ? d : NULL; + } + + static void domain_conn_reset(struct domain *domain) +@@ -399,15 +447,21 @@ int do_introduce(struct connection *conn, struct buffered_data *in) + if (port <= 0) + return EINVAL; + +- domain = find_domain_by_domid(domid); ++ domain = find_domain_struct(domid); + + if (domain == NULL) { ++ /* Hang domain off "in" until we're finished. */ ++ domain = alloc_domain(in, domid); ++ if (domain == NULL) ++ return ENOMEM; ++ } ++ ++ if (!domain->introduced) { + interface = map_interface(domid, mfn); + if (!interface) + return errno; + /* Hang domain off "in" until we're finished. */ +- domain = new_domain(in, domid, port); +- if (!domain) { ++ if (new_domain(domain, port)) { + rc = errno; + unmap_interface(interface); + return rc; +@@ -518,8 +572,8 @@ int do_resume(struct connection *conn, struct buffered_data *in) + if (IS_ERR(domain)) + return -PTR_ERR(domain); + +- domain->shutdown = 0; +- ++ domain->shutdown = false; ++ + send_ack(conn, XS_RESUME); + + return 0; +@@ -662,8 +716,10 @@ static int dom0_init(void) + if (port == -1) + return -1; + +- dom0 = new_domain(NULL, xenbus_master_domid(), port); +- if (dom0 == NULL) ++ dom0 = alloc_domain(NULL, xenbus_master_domid()); ++ if (!dom0) ++ return -1; ++ if (new_domain(dom0, port)) + return -1; + + dom0->interface = xenbus_map(); +@@ -744,6 +800,66 @@ void domain_entry_inc(struct connection *conn, struct node *node) + } + } + ++/* ++ * Check whether a domain was created before or after a specific generation ++ * count (used for testing whether a node permission is older than a domain). ++ * ++ * Return values: ++ * -1: error ++ * 0: domain has higher generation count (it is younger than a node with the ++ * given count), or domain isn't existing any longer ++ * 1: domain is older than the node ++ */ ++static int chk_domain_generation(unsigned int domid, uint64_t gen) ++{ ++ struct domain *d; ++ xc_dominfo_t dominfo; ++ ++ if (!xc_handle && domid == 0) ++ return 1; ++ ++ d = find_domain_struct(domid); ++ if (d) ++ return (d->generation <= gen) ? 1 : 0; ++ ++ if (!get_domain_info(domid, &dominfo)) ++ return 0; ++ ++ d = alloc_domain(NULL, domid); ++ return d ? 1 : -1; ++} ++ ++/* ++ * Remove permissions for no longer existing domains in order to avoid a new ++ * domain with the same domid inheriting the permissions. ++ */ ++int domain_adjust_node_perms(struct node *node) ++{ ++ unsigned int i; ++ int ret; ++ ++ ret = chk_domain_generation(node->perms.p[0].id, node->generation); ++ if (ret < 0) ++ return errno; ++ ++ /* If the owner doesn't exist any longer give it to priv domain. */ ++ if (!ret) ++ node->perms.p[0].id = priv_domid; ++ ++ for (i = 1; i < node->perms.num; i++) { ++ if (node->perms.p[i].perms & XS_PERM_IGNORE) ++ continue; ++ ret = chk_domain_generation(node->perms.p[i].id, ++ node->generation); ++ if (ret < 0) ++ return errno; ++ if (!ret) ++ node->perms.p[i].perms |= XS_PERM_IGNORE; ++ } ++ ++ return 0; ++} ++ + void domain_entry_dec(struct connection *conn, struct node *node) + { + struct domain *d; +diff --git a/tools/xenstore/xenstored_domain.h b/tools/xenstore/xenstored_domain.h +index 2591839..5e00087 100644 +--- a/tools/xenstore/xenstored_domain.h ++++ b/tools/xenstore/xenstored_domain.h +@@ -56,6 +56,9 @@ bool domain_can_write(struct connection *conn); + + bool domain_is_unprivileged(struct connection *conn); + ++/* Remove node permissions for no longer existing domains. */ ++int domain_adjust_node_perms(struct node *node); ++ + /* Quota manipulation */ + void domain_entry_inc(struct connection *conn, struct node *); + void domain_entry_dec(struct connection *conn, struct node *); +diff --git a/tools/xenstore/xenstored_transaction.c b/tools/xenstore/xenstored_transaction.c +index 36793b9..9fcb4c9 100644 +--- a/tools/xenstore/xenstored_transaction.c ++++ b/tools/xenstore/xenstored_transaction.c +@@ -47,7 +47,12 @@ + * transaction. + * Each time the global generation count is copied to either a node or a + * transaction it is incremented. This ensures all nodes and/or transactions +- * are having a unique generation count. ++ * are having a unique generation count. The increment is done _before_ the ++ * copy as that is needed for checking whether a domain was created before ++ * or after a node has been written (the domain's generation is set with the ++ * actual generation count without incrementing it, in order to support ++ * writing a node for a domain before the domain has been officially ++ * introduced). + * + * Transaction conflicts are detected by checking the generation count of all + * nodes read in the transaction to match with the generation count in the +@@ -161,7 +166,7 @@ struct transaction + }; + + extern int quota_max_transaction; +-static uint64_t generation; ++uint64_t generation; + + static void set_tdb_key(const char *name, TDB_DATA *key) + { +@@ -237,7 +242,7 @@ int access_node(struct connection *conn, struct node *node, + bool introduce = false; + + if (type != NODE_ACCESS_READ) { +- node->generation = generation++; ++ node->generation = ++generation; + if (conn && !conn->transaction) + wrl_apply_debit_direct(conn); + } +@@ -374,7 +379,7 @@ static int finalize_transaction(struct connection *conn, + if (!data.dptr) + goto err; + hdr = (void *)data.dptr; +- hdr->generation = generation++; ++ hdr->generation = ++generation; + ret = tdb_store(tdb_ctx, key, data, + TDB_REPLACE); + talloc_free(data.dptr); +@@ -462,7 +467,7 @@ int do_transaction_start(struct connection *conn, struct buffered_data *in) + INIT_LIST_HEAD(&trans->accessed); + INIT_LIST_HEAD(&trans->changed_domains); + trans->fail = false; +- trans->generation = generation++; ++ trans->generation = ++generation; + + /* Pick an unused transaction identifier. */ + do { +diff --git a/tools/xenstore/xenstored_transaction.h b/tools/xenstore/xenstored_transaction.h +index 3386bac..43a162b 100644 +--- a/tools/xenstore/xenstored_transaction.h ++++ b/tools/xenstore/xenstored_transaction.h +@@ -27,6 +27,8 @@ enum node_access_type { + + struct transaction; + ++extern uint64_t generation; ++ + int do_transaction_start(struct connection *conn, struct buffered_data *node); + int do_transaction_end(struct connection *conn, struct buffered_data *in); + +diff --git a/tools/xenstore/xs_lib.c b/tools/xenstore/xs_lib.c +index 3e43f88..d407d57 100644 +--- a/tools/xenstore/xs_lib.c ++++ b/tools/xenstore/xs_lib.c +@@ -152,7 +152,7 @@ bool xs_strings_to_perms(struct xs_permissions *perms, unsigned int num, + bool xs_perm_to_string(const struct xs_permissions *perm, + char *buffer, size_t buf_len) + { +- switch ((int)perm->perms) { ++ switch ((int)perm->perms & ~XS_PERM_IGNORE) { + case XS_PERM_WRITE: + *buffer = 'w'; + break; diff -Nru xen-4.11.4+57-g41a822c392/debian/patches/0068-tools-ocaml-xenstored-clean-up-permissions-for-dead-.patch xen-4.11.4+57-g41a822c392/debian/patches/0068-tools-ocaml-xenstored-clean-up-permissions-for-dead-.patch --- xen-4.11.4+57-g41a822c392/debian/patches/0068-tools-ocaml-xenstored-clean-up-permissions-for-dead-.patch 1970-01-01 00:00:00.000000000 +0000 +++ xen-4.11.4+57-g41a822c392/debian/patches/0068-tools-ocaml-xenstored-clean-up-permissions-for-dead-.patch 2020-12-11 21:10:09.000000000 +0000 @@ -0,0 +1,117 @@ +From: =?utf-8?b?RWR3aW4gVMO2csO2aw==?= +Date: Fri, 11 Dec 2020 21:51:30 +0100 +Subject: tools/ocaml/xenstored: clean up permissions for dead domains +MIME-Version: 1.0 +Content-Type: text/plain; charset="utf-8" +Content-Transfer-Encoding: 8bit + +domain ids are prone to wrapping (15-bits), and with sufficient number +of VMs in a reboot loop it is possible to trigger it. Xenstore entries +may linger after a domain dies, until a toolstack cleans it up. During +this time there is a window where a wrapped domid could access these +xenstore keys (that belonged to another VM). + +To prevent this do a cleanup when a domain dies: + * walk the entire xenstore tree and update permissions for all nodes + * if the dead domain had an ACL entry: remove it + * if the dead domain was the owner: change the owner to Dom0 + +This is done without quota checks or a transaction. Quota checks would +be a no-op (either the domain is dead, or it is Dom0 where they are not +enforced). Transactions are not needed, because this is all done +atomically by oxenstored's single thread. + +The xenstore entries owned by the dead domain are not deleted, because +that could confuse a toolstack / backends that are still bound to it +(or generate unexpected watch events). It is the responsibility of a +toolstack to remove the xenstore entries themselves. + +This is part of XSA-322. + +Signed-off-by: Edwin Török +Acked-by: Christian Lindig +--- + tools/ocaml/xenstored/perms.ml | 9 +++++++++ + tools/ocaml/xenstored/process.ml | 1 + + tools/ocaml/xenstored/store.ml | 16 ++++++++++++++++ + tools/ocaml/xenstored/xenstored.ml | 1 + + 4 files changed, 27 insertions(+) + +diff --git a/tools/ocaml/xenstored/perms.ml b/tools/ocaml/xenstored/perms.ml +index ee7fee6..e8a1622 100644 +--- a/tools/ocaml/xenstored/perms.ml ++++ b/tools/ocaml/xenstored/perms.ml +@@ -58,6 +58,15 @@ let get_other perms = perms.other + let get_acl perms = perms.acl + let get_owner perm = perm.owner + ++(** [remote_domid ~domid perm] removes all ACLs for [domid] from perm. ++* If [domid] was the owner then it is changed to Dom0. ++* This is used for cleaning up after dead domains. ++* *) ++let remove_domid ~domid perm = ++ let acl = List.filter (fun (acl_domid, _) -> acl_domid <> domid) perm.acl in ++ let owner = if perm.owner = domid then 0 else perm.owner in ++ { perm with acl; owner } ++ + let default0 = create 0 NONE [] + + let perm_of_string s = +diff --git a/tools/ocaml/xenstored/process.ml b/tools/ocaml/xenstored/process.ml +index 3cd0097..6a998f8 100644 +--- a/tools/ocaml/xenstored/process.ml ++++ b/tools/ocaml/xenstored/process.ml +@@ -437,6 +437,7 @@ let do_release con t domains cons data = + let fire_spec_watches = Domains.exist domains domid in + Domains.del domains domid; + Connections.del_domain cons domid; ++ Store.reset_permissions (Transaction.get_store t) domid; + if fire_spec_watches + then Connections.fire_spec_watches (Transaction.get_root t) cons Store.Path.release_domain + else raise Invalid_Cmd_Args +diff --git a/tools/ocaml/xenstored/store.ml b/tools/ocaml/xenstored/store.ml +index 98d368d..6bbbd05 100644 +--- a/tools/ocaml/xenstored/store.ml ++++ b/tools/ocaml/xenstored/store.ml +@@ -89,6 +89,13 @@ let check_owner node connection = + + let rec recurse fct node = fct node; List.iter (recurse fct) node.children + ++(** [recurse_map f tree] applies [f] on each node in the tree recursively *) ++let recurse_map f = ++ let rec walk node = ++ f { node with children = List.rev_map walk node.children |> List.rev } ++ in ++ walk ++ + let unpack node = (Symbol.to_string node.name, node.perms, node.value) + + end +@@ -435,6 +442,15 @@ let setperms store perm path nperms = + Quota.del_entry store.quota old_owner; + Quota.add_entry store.quota new_owner + ++let reset_permissions store domid = ++ Logging.info "store|node" "Cleaning up xenstore ACLs for domid %d" domid; ++ store.root <- Node.recurse_map (fun node -> ++ let perms = Perms.Node.remove_domid ~domid node.perms in ++ if perms <> node.perms then ++ Logging.debug "store|node" "Changed permissions for node %s" (Node.get_name node); ++ { node with perms } ++ ) store.root ++ + type ops = { + store: t; + write: Path.t -> string -> unit; +diff --git a/tools/ocaml/xenstored/xenstored.ml b/tools/ocaml/xenstored/xenstored.ml +index 30fc874..183dd27 100644 +--- a/tools/ocaml/xenstored/xenstored.ml ++++ b/tools/ocaml/xenstored/xenstored.ml +@@ -340,6 +340,7 @@ let _ = + finally (fun () -> + if Some port = eventchn.Event.virq_port then ( + let (notify, deaddom) = Domains.cleanup domains in ++ List.iter (Store.reset_permissions store) deaddom; + List.iter (Connections.del_domain cons) deaddom; + if deaddom <> [] || notify then + Connections.fire_spec_watches diff -Nru xen-4.11.4+57-g41a822c392/debian/patches/0069-tools-ocaml-xenstored-Fix-path-length-validation.patch xen-4.11.4+57-g41a822c392/debian/patches/0069-tools-ocaml-xenstored-Fix-path-length-validation.patch --- xen-4.11.4+57-g41a822c392/debian/patches/0069-tools-ocaml-xenstored-Fix-path-length-validation.patch 1970-01-01 00:00:00.000000000 +0000 +++ xen-4.11.4+57-g41a822c392/debian/patches/0069-tools-ocaml-xenstored-Fix-path-length-validation.patch 2020-12-11 21:10:09.000000000 +0000 @@ -0,0 +1,150 @@ +From: =?utf-8?b?RWR3aW4gVMO2csO2aw==?= +Date: Fri, 11 Dec 2020 21:52:01 +0100 +Subject: tools/ocaml/xenstored: Fix path length validation +MIME-Version: 1.0 +Content-Type: text/plain; charset="utf-8" +Content-Transfer-Encoding: 8bit + +Currently, oxenstored checks the length of paths against 1024, then +prepends "/local/domain/$DOMID/" to relative paths. This allows a domU +to create paths which can't subsequently be read by anyone, even dom0. +This also interferes with listing directories, etc. + +Define a new oxenstored.conf entry: quota-path-max, defaulting to 1024 +as before. For paths that begin with "/local/domain/$DOMID/" check the +relative path length against this quota. For all other paths check the +entire path length. + +This ensures that if the domid changes (and thus the length of a prefix +changes) a path that used to be valid stays valid (e.g. after a +live-migration). It also ensures that regardless how the client tries +to access a path (domid-relative or absolute) it will get consistent +results, since the limit is always applied on the final canonicalized +path. + +Delete the unused Domain.get_path to avoid it being confused with +Connection.get_path (which differs by a trailing slash only). + +Rewrite Util.path_validate to apply the appropriate length restriction +based on whether the path is relative or not. Remove the check for +connection_path being absolute, because it is not guest controlled data. + +This is part of XSA-323. + +Signed-off-by: Andrew Cooper +Signed-off-by: Edwin Török +Acked-by: Christian Lindig +--- + tools/ocaml/libs/xb/partial.ml | 1 + + tools/ocaml/libs/xb/partial.mli | 1 + + tools/ocaml/xenstored/define.ml | 2 ++ + tools/ocaml/xenstored/domain.ml | 1 - + tools/ocaml/xenstored/oxenstored.conf.in | 1 + + tools/ocaml/xenstored/utils.ml | 15 ++++++++++++++- + tools/ocaml/xenstored/xenstored.ml | 1 + + 7 files changed, 20 insertions(+), 2 deletions(-) + +diff --git a/tools/ocaml/libs/xb/partial.ml b/tools/ocaml/libs/xb/partial.ml +index d4d1c7b..b6e2a71 100644 +--- a/tools/ocaml/libs/xb/partial.ml ++++ b/tools/ocaml/libs/xb/partial.ml +@@ -28,6 +28,7 @@ external header_of_string_internal: string -> int * int * int * int + = "stub_header_of_string" + + let xenstore_payload_max = 4096 (* xen/include/public/io/xs_wire.h *) ++let xenstore_rel_path_max = 2048 (* xen/include/public/io/xs_wire.h *) + + let of_string s = + let tid, rid, opint, dlen = header_of_string_internal s in +diff --git a/tools/ocaml/libs/xb/partial.mli b/tools/ocaml/libs/xb/partial.mli +index 359a75e..b921601 100644 +--- a/tools/ocaml/libs/xb/partial.mli ++++ b/tools/ocaml/libs/xb/partial.mli +@@ -9,6 +9,7 @@ external header_size : unit -> int = "stub_header_size" + external header_of_string_internal : string -> int * int * int * int + = "stub_header_of_string" + val xenstore_payload_max : int ++val xenstore_rel_path_max : int + val of_string : string -> pkt + val append : pkt -> string -> int -> unit + val to_complete : pkt -> int +diff --git a/tools/ocaml/xenstored/define.ml b/tools/ocaml/xenstored/define.ml +index 2965c08..f574397 100644 +--- a/tools/ocaml/xenstored/define.ml ++++ b/tools/ocaml/xenstored/define.ml +@@ -32,6 +32,8 @@ let conflict_rate_limit_is_aggregate = ref true + + let domid_self = 0x7FF0 + ++let path_max = ref Xenbus.Partial.xenstore_rel_path_max ++ + exception Not_a_directory of string + exception Not_a_value of string + exception Already_exist +diff --git a/tools/ocaml/xenstored/domain.ml b/tools/ocaml/xenstored/domain.ml +index b0a01b0..0810762 100644 +--- a/tools/ocaml/xenstored/domain.ml ++++ b/tools/ocaml/xenstored/domain.ml +@@ -38,7 +38,6 @@ type t = + } + + let is_dom0 d = d.id = 0 +-let get_path dom = "/local/domain/" ^ (sprintf "%u" dom.id) + let get_id domain = domain.id + let get_interface d = d.interface + let get_mfn d = d.mfn +diff --git a/tools/ocaml/xenstored/oxenstored.conf.in b/tools/ocaml/xenstored/oxenstored.conf.in +index d5d4f00..bef6330 100644 +--- a/tools/ocaml/xenstored/oxenstored.conf.in ++++ b/tools/ocaml/xenstored/oxenstored.conf.in +@@ -61,6 +61,7 @@ quota-maxsize = 2048 + quota-maxwatch = 100 + quota-transaction = 10 + quota-maxrequests = 1024 ++quota-path-max = 1024 + + # Activate filed base backend + persistent = false +diff --git a/tools/ocaml/xenstored/utils.ml b/tools/ocaml/xenstored/utils.ml +index e8c9fe4..eb79bf0 100644 +--- a/tools/ocaml/xenstored/utils.ml ++++ b/tools/ocaml/xenstored/utils.ml +@@ -93,7 +93,7 @@ let read_file_single_integer filename = + let path_validate path connection_path = + let len = String.length path in + +- if len = 0 || len > 1024 then raise Define.Invalid_path; ++ if len = 0 then raise Define.Invalid_path; + + let abs_path = + match String.get path 0 with +@@ -101,4 +101,17 @@ let path_validate path connection_path = + | _ -> connection_path ^ path + in + ++ (* Regardless whether client specified absolute or relative path, ++ canonicalize it (above) and, for domain-relative paths, check the ++ length of the relative part. ++ ++ This prevents paths becoming invalid across migrate when the length ++ of the domid changes in @param connection_path. ++ *) ++ let len = String.length abs_path in ++ let on_absolute _ _ = len in ++ let on_relative _ offset = len - offset in ++ let len = Scanf.ksscanf abs_path on_absolute "/local/domain/%d/%n" on_relative in ++ if len > !Define.path_max then raise Define.Invalid_path; ++ + abs_path +diff --git a/tools/ocaml/xenstored/xenstored.ml b/tools/ocaml/xenstored/xenstored.ml +index 183dd27..70f1bf8 100644 +--- a/tools/ocaml/xenstored/xenstored.ml ++++ b/tools/ocaml/xenstored/xenstored.ml +@@ -102,6 +102,7 @@ let parse_config filename = + ("quota-maxentity", Config.Set_int Quota.maxent); + ("quota-maxsize", Config.Set_int Quota.maxsize); + ("quota-maxrequests", Config.Set_int Define.maxrequests); ++ ("quota-path-max", Config.Set_int Define.path_max); + ("test-eagain", Config.Set_bool Transaction.test_eagain); + ("persistent", Config.Set_bool Disk.enable); + ("xenstored-log-file", Config.String Logging.set_xenstored_log_destination); diff -Nru xen-4.11.4+57-g41a822c392/debian/patches/0070-tools-xenstore-drop-watch-event-messages-exceeding-m.patch xen-4.11.4+57-g41a822c392/debian/patches/0070-tools-xenstore-drop-watch-event-messages-exceeding-m.patch --- xen-4.11.4+57-g41a822c392/debian/patches/0070-tools-xenstore-drop-watch-event-messages-exceeding-m.patch 1970-01-01 00:00:00.000000000 +0000 +++ xen-4.11.4+57-g41a822c392/debian/patches/0070-tools-xenstore-drop-watch-event-messages-exceeding-m.patch 2020-12-11 21:10:09.000000000 +0000 @@ -0,0 +1,53 @@ +From: Juergen Gross +Date: Fri, 11 Dec 2020 21:53:29 +0100 +Subject: tools/xenstore: drop watch event messages exceeding maximum size + +By setting a watch with a very large tag it is possible to trick +xenstored to send watch event messages exceeding the maximum allowed +payload size. This might in turn lead to a crash of xenstored as the +resulting error can cause dereferencing a NULL pointer in case there +is no active request being handled by the guest the watch event is +being sent to. + +Fix that by just dropping such watch events. Additionally modify the +error handling to test the pointer to be not NULL before dereferencing +it. + +This is XSA-324. + +Signed-off-by: Juergen Gross +Acked-by: Julien Grall +--- + tools/xenstore/xenstored_core.c | 3 +++ + tools/xenstore/xenstored_watch.c | 4 ++++ + 2 files changed, 7 insertions(+) + +diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c +index 4fbe5c7..e8f2057 100644 +--- a/tools/xenstore/xenstored_core.c ++++ b/tools/xenstore/xenstored_core.c +@@ -680,6 +680,9 @@ void send_reply(struct connection *conn, enum xsd_sockmsg_type type, + /* Replies reuse the request buffer, events need a new one. */ + if (type != XS_WATCH_EVENT) { + bdata = conn->in; ++ /* Drop asynchronous responses, e.g. errors for watch events. */ ++ if (!bdata) ++ return; + bdata->inhdr = true; + bdata->used = 0; + conn->in = NULL; +diff --git a/tools/xenstore/xenstored_watch.c b/tools/xenstore/xenstored_watch.c +index be24797..b2b77a3 100644 +--- a/tools/xenstore/xenstored_watch.c ++++ b/tools/xenstore/xenstored_watch.c +@@ -92,6 +92,10 @@ static void add_event(struct connection *conn, + } + + len = strlen(name) + 1 + strlen(watch->token) + 1; ++ /* Don't try to send over-long events. */ ++ if (len > XENSTORE_PAYLOAD_MAX) ++ return; ++ + data = talloc_array(ctx, char, len); + if (!data) + return; diff -Nru xen-4.11.4+57-g41a822c392/debian/patches/0071-tools-xenstore-Preserve-bad-client-until-they-are-de.patch xen-4.11.4+57-g41a822c392/debian/patches/0071-tools-xenstore-Preserve-bad-client-until-they-are-de.patch --- xen-4.11.4+57-g41a822c392/debian/patches/0071-tools-xenstore-Preserve-bad-client-until-they-are-de.patch 1970-01-01 00:00:00.000000000 +0000 +++ xen-4.11.4+57-g41a822c392/debian/patches/0071-tools-xenstore-Preserve-bad-client-until-they-are-de.patch 2020-12-11 21:10:09.000000000 +0000 @@ -0,0 +1,194 @@ +From: Harsha Shamsundara Havanur +Date: Fri, 11 Dec 2020 21:55:23 +0100 +Subject: tools/xenstore: Preserve bad client until they are destroyed + +XenStored will kill any connection that it thinks has misbehaved, +this is currently happening in two places: + * In `handle_input()` if the sanity check on the ring and the message + fails. + * In `handle_output()` when failing to write the response in the ring. + +As the domain structure is a child of the connection, XenStored will +destroy its view of the domain when killing the connection. This will +result in sending @releaseDomain event to all the watchers. + +As the watch event doesn't carry which domain has been released, +the watcher (such as XenStored) will generally go through the list of +domains registers and check if one of them is shutting down/dying. +In the case of a client misbehaving, the domain will likely to be +running, so no action will be performed. + +When the domain is effectively destroyed, XenStored will not be aware of +the domain anymore. So the watch event is not going to be sent. +By consequence, the watchers of the event will not release mappings +they may have on the domain. This will result in a zombie domain. + +In order to send @releaseDomain event at the correct time, we want +to keep the domain structure until the domain is effectively +shutting-down/dying. + +We also want to keep the connection around so we could possibly revive +the connection in the future. + +A new flag 'is_ignored' is added to mark whether a connection should be +ignored when checking if there are work to do. Additionally any +transactions, watches, buffers associated to the connection will be +freed as you can't do much with them (restarting the connection will +likely need a reset). + +As a side note, when the device model were running in a stubdomain, a +guest would have been able to introduce a use-after-free because there +is two parents for a guest connection. + +This is XSA-325. + +Signed-off-by: Harsha Shamsundara Havanur +Signed-off-by: Julien Grall +Reviewed-by: Juergen Gross +Reviewed-by: Paul Durrant +--- + tools/xenstore/xenstored_core.c | 49 ++++++++++++++++++++++++++++++++------- + tools/xenstore/xenstored_core.h | 3 +++ + tools/xenstore/xenstored_domain.c | 8 +++++++ + 3 files changed, 51 insertions(+), 9 deletions(-) + +diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c +index e8f2057..0737c55 100644 +--- a/tools/xenstore/xenstored_core.c ++++ b/tools/xenstore/xenstored_core.c +@@ -1344,6 +1344,32 @@ static struct { + [XS_DIRECTORY_PART] = { "DIRECTORY_PART", send_directory_part }, + }; + ++/* ++ * Keep the connection alive but stop processing any new request or sending ++ * reponse. This is to allow sending @releaseDomain watch event at the correct ++ * moment and/or to allow the connection to restart (not yet implemented). ++ * ++ * All watches, transactions, buffers will be freed. ++ */ ++static void ignore_connection(struct connection *conn) ++{ ++ struct buffered_data *out, *tmp; ++ ++ trace("CONN %p ignored\n", conn); ++ ++ conn->is_ignored = true; ++ conn_delete_all_watches(conn); ++ conn_delete_all_transactions(conn); ++ ++ list_for_each_entry_safe(out, tmp, &conn->out_list, list) { ++ list_del(&out->list); ++ talloc_free(out); ++ } ++ ++ talloc_free(conn->in); ++ conn->in = NULL; ++} ++ + static const char *sockmsg_string(enum xsd_sockmsg_type type) + { + if ((unsigned int)type < ARRAY_SIZE(wire_funcs) && wire_funcs[type].str) +@@ -1402,8 +1428,10 @@ static void consider_message(struct connection *conn) + assert(conn->in == NULL); + } + +-/* Errors in reading or allocating here mean we get out of sync, so we +- * drop the whole client connection. */ ++/* ++ * Errors in reading or allocating here means we get out of sync, so we mark ++ * the connection as ignored. ++ */ + static void handle_input(struct connection *conn) + { + int bytes; +@@ -1460,14 +1488,14 @@ static void handle_input(struct connection *conn) + return; + + bad_client: +- /* Kill it. */ +- talloc_free(conn); ++ ignore_connection(conn); + } + + static void handle_output(struct connection *conn) + { ++ /* Ignore the connection if an error occured */ + if (!write_messages(conn)) +- talloc_free(conn); ++ ignore_connection(conn); + } + + struct connection *new_connection(connwritefn_t *write, connreadfn_t *read) +@@ -1483,6 +1511,7 @@ struct connection *new_connection(connwritefn_t *write, connreadfn_t *read) + new->write = write; + new->read = read; + new->can_write = true; ++ new->is_ignored = false; + new->transaction_started = 0; + INIT_LIST_HEAD(&new->out_list); + INIT_LIST_HEAD(&new->watches); +@@ -2185,8 +2214,9 @@ int main(int argc, char *argv[]) + if (fds[conn->pollfd_idx].revents + & ~(POLLIN|POLLOUT)) + talloc_free(conn); +- else if (fds[conn->pollfd_idx].revents +- & POLLIN) ++ else if ((fds[conn->pollfd_idx].revents ++ & POLLIN) && ++ !conn->is_ignored) + handle_input(conn); + } + if (talloc_free(conn) == 0) +@@ -2198,8 +2228,9 @@ int main(int argc, char *argv[]) + if (fds[conn->pollfd_idx].revents + & ~(POLLIN|POLLOUT)) + talloc_free(conn); +- else if (fds[conn->pollfd_idx].revents +- & POLLOUT) ++ else if ((fds[conn->pollfd_idx].revents ++ & POLLOUT) && ++ !conn->is_ignored) + handle_output(conn); + } + if (talloc_free(conn) == 0) +diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h +index e4966c8..6d1d046 100644 +--- a/tools/xenstore/xenstored_core.h ++++ b/tools/xenstore/xenstored_core.h +@@ -82,6 +82,9 @@ struct connection + /* Is this a read-only connection? */ + bool can_write; + ++ /* Is this connection ignored? */ ++ bool is_ignored; ++ + /* Buffered incoming data. */ + struct buffered_data *in; + +diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c +index f5e7af4..44562e8 100644 +--- a/tools/xenstore/xenstored_domain.c ++++ b/tools/xenstore/xenstored_domain.c +@@ -298,6 +298,10 @@ bool domain_can_read(struct connection *conn) + + if (domain_is_unprivileged(conn) && conn->domain->wrl_credit < 0) + return false; ++ ++ if (conn->is_ignored) ++ return false; ++ + return (intf->req_cons != intf->req_prod); + } + +@@ -315,6 +319,10 @@ bool domain_is_unprivileged(struct connection *conn) + bool domain_can_write(struct connection *conn) + { + struct xenstore_domain_interface *intf = conn->domain->interface; ++ ++ if (conn->is_ignored) ++ return false; ++ + return ((intf->rsp_prod - intf->rsp_cons) != XENSTORE_RING_SIZE); + } + diff -Nru xen-4.11.4+57-g41a822c392/debian/patches/0072-tools-ocaml-xenstored-delete-watch-from-trie-too-whe.patch xen-4.11.4+57-g41a822c392/debian/patches/0072-tools-ocaml-xenstored-delete-watch-from-trie-too-whe.patch --- xen-4.11.4+57-g41a822c392/debian/patches/0072-tools-ocaml-xenstored-delete-watch-from-trie-too-whe.patch 1970-01-01 00:00:00.000000000 +0000 +++ xen-4.11.4+57-g41a822c392/debian/patches/0072-tools-ocaml-xenstored-delete-watch-from-trie-too-whe.patch 2020-12-11 21:10:09.000000000 +0000 @@ -0,0 +1,71 @@ +From: =?utf-8?b?RWR3aW4gVMO2csO2aw==?= +Date: Fri, 11 Dec 2020 21:56:22 +0100 +Subject: tools/ocaml/xenstored: delete watch from trie too when resetting + watches +MIME-Version: 1.0 +Content-Type: text/plain; charset="utf-8" +Content-Transfer-Encoding: 8bit + +c/s f8c72b526129 "oxenstored: implement XS_RESET_WATCHES" from Xen 4.6 +introduced reset watches support in oxenstored by mirroring the change +in cxenstored. + +However the OCaml version has some additional data structures to +optimize watch firing, and just resetting the watches in one of the data +structures creates a security bug where a malicious guest kernel can +exceed its watch quota, driving oxenstored into OOM: + * create watches + * reset watches (this still keeps the watches lingering in another data + structure, using memory) + * create some more watches + * loop until oxenstored dies + +The guest kernel doesn't necessarily have to be malicious to trigger +this: + * if control/platform-feature-xs_reset_watches is set + * the guest kexecs (e.g. because it crashes) + * on boot more watches are set up + * this will slowly "leak" memory for watches in oxenstored, driving it + towards OOM. + +This is XSA-330. + +Fixes: f8c72b526129 ("oxenstored: implement XS_RESET_WATCHES") +Signed-off-by: Edwin Török +Acked-by: Christian Lindig +Reviewed-by: Andrew Cooper +--- + tools/ocaml/xenstored/connections.ml | 4 ++++ + tools/ocaml/xenstored/process.ml | 4 ++-- + 2 files changed, 6 insertions(+), 2 deletions(-) + +diff --git a/tools/ocaml/xenstored/connections.ml b/tools/ocaml/xenstored/connections.ml +index 020b875..4e69de1 100644 +--- a/tools/ocaml/xenstored/connections.ml ++++ b/tools/ocaml/xenstored/connections.ml +@@ -134,6 +134,10 @@ let del_watch cons con path token = + cons.watches <- Trie.set cons.watches key watches; + watch + ++let del_watches cons con = ++ Connection.del_watches con; ++ cons.watches <- Trie.map (del_watches_of_con con) cons.watches ++ + (* path is absolute *) + let fire_watches ?oldroot root cons path recurse = + let key = key_of_path path in +diff --git a/tools/ocaml/xenstored/process.ml b/tools/ocaml/xenstored/process.ml +index 6a998f8..12ad66f 100644 +--- a/tools/ocaml/xenstored/process.ml ++++ b/tools/ocaml/xenstored/process.ml +@@ -179,8 +179,8 @@ let do_isintroduced con _t domains _cons data = + if domid = Define.domid_self || Domains.exist domains domid then "T\000" else "F\000" + + (* only in xen >= 4.2 *) +-let do_reset_watches con t domains cons data = +- Connection.del_watches con; ++let do_reset_watches con _t _domains cons _data = ++ Connections.del_watches cons con; + Connection.del_transactions con + + (* only in >= xen3.3 *) diff -Nru xen-4.11.4+57-g41a822c392/debian/patches/0073-tools-ocaml-xenstored-only-Dom0-can-change-node-owne.patch xen-4.11.4+57-g41a822c392/debian/patches/0073-tools-ocaml-xenstored-only-Dom0-can-change-node-owne.patch --- xen-4.11.4+57-g41a822c392/debian/patches/0073-tools-ocaml-xenstored-only-Dom0-can-change-node-owne.patch 1970-01-01 00:00:00.000000000 +0000 +++ xen-4.11.4+57-g41a822c392/debian/patches/0073-tools-ocaml-xenstored-only-Dom0-can-change-node-owne.patch 2020-12-11 21:10:09.000000000 +0000 @@ -0,0 +1,46 @@ +From: =?utf-8?b?RWR3aW4gVMO2csO2aw==?= +Date: Fri, 11 Dec 2020 21:58:00 +0100 +Subject: tools/ocaml/xenstored: only Dom0 can change node owner +MIME-Version: 1.0 +Content-Type: text/plain; charset="utf-8" +Content-Transfer-Encoding: 8bit + +Otherwise we can give quota away to another domain, either causing it to run +out of quota, or in case of Dom0 use unbounded amounts of memory and bypass +the quota system entirely. + +This was fixed in the C version of xenstored in 2006 (c/s db34d2aaa5f5, +predating the XSA process by 5 years). + +It was also fixed in the mirage version of xenstore in 2012, with a unit test +demonstrating the vulnerability: + + https://github.com/mirage/ocaml-xenstore/commit/6b91f3ac46b885d0530a51d57a9b3a57d64923a7 + https://github.com/mirage/ocaml-xenstore/commit/22ee5417c90b8fda905c38de0d534506152eace6 + +but possibly without realising that the vulnerability still affected the +in-tree oxenstored (added c/s f44af660412 in 2010). + +This is XSA-352. + +Signed-off-by: Edwin Török +Acked-by: Christian Lindig +Reviewed-by: Andrew Cooper +--- + tools/ocaml/xenstored/store.ml | 3 ++- + 1 file changed, 2 insertions(+), 1 deletion(-) + +diff --git a/tools/ocaml/xenstored/store.ml b/tools/ocaml/xenstored/store.ml +index 6bbbd05..2a69456 100644 +--- a/tools/ocaml/xenstored/store.ml ++++ b/tools/ocaml/xenstored/store.ml +@@ -437,7 +437,8 @@ let setperms store perm path nperms = + | Some node -> + let old_owner = Node.get_owner node in + let new_owner = Perms.Node.get_owner nperms in +- if not ((old_owner = new_owner) || (Perms.Connection.is_dom0 perm)) then Quota.check store.quota new_owner 0; ++ if not ((old_owner = new_owner) || (Perms.Connection.is_dom0 perm)) then ++ raise Define.Permission_denied; + store.root <- path_setperms store perm path nperms; + Quota.del_entry store.quota old_owner; + Quota.add_entry store.quota new_owner diff -Nru xen-4.11.4+57-g41a822c392/debian/patches/0074-x86-avoid-calling-svm-vmx-_do_resume.patch xen-4.11.4+57-g41a822c392/debian/patches/0074-x86-avoid-calling-svm-vmx-_do_resume.patch --- xen-4.11.4+57-g41a822c392/debian/patches/0074-x86-avoid-calling-svm-vmx-_do_resume.patch 1970-01-01 00:00:00.000000000 +0000 +++ xen-4.11.4+57-g41a822c392/debian/patches/0074-x86-avoid-calling-svm-vmx-_do_resume.patch 2020-12-11 21:10:09.000000000 +0000 @@ -0,0 +1,188 @@ +From: Jan Beulich +Date: Fri, 11 Dec 2020 21:59:07 +0100 +Subject: x86: avoid calling {svm,vmx}_do_resume() + +These functions follow the following path: hvm_do_resume() -> +handle_hvm_io_completion() -> hvm_wait_for_io() -> +wait_on_xen_event_channel() -> do_softirq() -> schedule() -> +sched_context_switch() -> continue_running() and hence may +recursively invoke themselves. If this ends up happening a couple of +times, a stack overflow would result. + +Prevent this by also resetting the stack at the +->arch.ctxt_switch->tail() invocations (in both places for consistency) +and thus jumping to the functions instead of calling them. + +This is XSA-348 / CVE-2020-29566. + +Reported-by: Julien Grall +Signed-off-by: Jan Beulich +Reviewed-by: Juergen Gross +--- + xen/arch/x86/domain.c | 21 ++++----------------- + xen/arch/x86/hvm/svm/svm.c | 3 ++- + xen/arch/x86/hvm/vmx/vmcs.c | 3 ++- + xen/arch/x86/pv/domain.c | 2 +- + xen/include/asm-x86/current.h | 15 +++++++++++---- + xen/include/asm-x86/domain.h | 2 +- + xen/include/asm-x86/hvm/vmx/vmx.h | 2 +- + 7 files changed, 22 insertions(+), 26 deletions(-) + +diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c +index 35857db..5182ca3 100644 +--- a/xen/arch/x86/domain.c ++++ b/xen/arch/x86/domain.c +@@ -121,7 +121,7 @@ static void play_dead(void) + (*dead_idle)(); + } + +-static void idle_loop(void) ++static void noreturn idle_loop(void) + { + unsigned int cpu = smp_processor_id(); + +@@ -161,11 +161,6 @@ void startup_cpu_idle_loop(void) + reset_stack_and_jump(idle_loop); + } + +-static void noreturn continue_idle_domain(struct vcpu *v) +-{ +- reset_stack_and_jump(idle_loop); +-} +- + void dump_pageframe_info(struct domain *d) + { + struct page_info *page; +@@ -456,7 +451,7 @@ int arch_domain_create(struct domain *d, + static const struct arch_csw idle_csw = { + .from = paravirt_ctxt_switch_from, + .to = paravirt_ctxt_switch_to, +- .tail = continue_idle_domain, ++ .tail = idle_loop, + }; + + d->arch.ctxt_switch = &idle_csw; +@@ -1764,20 +1759,12 @@ void context_switch(struct vcpu *prev, struct vcpu *next) + /* Ensure that the vcpu has an up-to-date time base. */ + update_vcpu_system_time(next); + +- /* +- * Schedule tail *should* be a terminal function pointer, but leave a +- * bug frame around just in case it returns, to save going back into the +- * context switching code and leaving a far more subtle crash to diagnose. +- */ +- nextd->arch.ctxt_switch->tail(next); +- BUG(); ++ reset_stack_and_jump_ind(nextd->arch.ctxt_switch->tail); + } + + void continue_running(struct vcpu *same) + { +- /* See the comment above. */ +- same->domain->arch.ctxt_switch->tail(same); +- BUG(); ++ reset_stack_and_jump_ind(same->domain->arch.ctxt_switch->tail); + } + + int __sync_local_execstate(void) +diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c +index 2f8aed8..8adc39c 100644 +--- a/xen/arch/x86/hvm/svm/svm.c ++++ b/xen/arch/x86/hvm/svm/svm.c +@@ -1111,8 +1111,9 @@ static void svm_ctxt_switch_to(struct vcpu *v) + wrmsr_tsc_aux(hvm_msr_tsc_aux(v)); + } + +-static void noreturn svm_do_resume(struct vcpu *v) ++static void noreturn svm_do_resume(void) + { ++ struct vcpu *v = current; + struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb; + bool debug_state = (v->domain->debugger_attached || + v->domain->arch.monitor.software_breakpoint_enabled || +diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c +index 332f2d8..e2019aa 100644 +--- a/xen/arch/x86/hvm/vmx/vmcs.c ++++ b/xen/arch/x86/hvm/vmx/vmcs.c +@@ -1782,8 +1782,9 @@ void vmx_vmentry_failure(void) + domain_crash_synchronous(); + } + +-void vmx_do_resume(struct vcpu *v) ++void vmx_do_resume(void) + { ++ struct vcpu *v = current; + bool_t debug_state; + unsigned long host_cr4; + +diff --git a/xen/arch/x86/pv/domain.c b/xen/arch/x86/pv/domain.c +index c6461cd..5d373c3 100644 +--- a/xen/arch/x86/pv/domain.c ++++ b/xen/arch/x86/pv/domain.c +@@ -58,7 +58,7 @@ static int parse_pcid(const char *s) + } + custom_runtime_param("pcid", parse_pcid); + +-static void noreturn continue_nonidle_domain(struct vcpu *v) ++static void noreturn continue_nonidle_domain(void) + { + check_wakeup_from_wait(); + reset_stack_and_jump(ret_from_intr); +diff --git a/xen/include/asm-x86/current.h b/xen/include/asm-x86/current.h +index f3508c3..cab3c6f 100644 +--- a/xen/include/asm-x86/current.h ++++ b/xen/include/asm-x86/current.h +@@ -124,16 +124,23 @@ unsigned long get_stack_dump_bottom (unsigned long sp); + # define CHECK_FOR_LIVEPATCH_WORK "" + #endif + +-#define reset_stack_and_jump(__fn) \ ++#define switch_stack_and_jump(fn, instr, constr) \ + ({ \ + __asm__ __volatile__ ( \ + "mov %0,%%"__OP"sp;" \ +- CHECK_FOR_LIVEPATCH_WORK \ +- "jmp %c1" \ +- : : "r" (guest_cpu_user_regs()), "i" (__fn) : "memory" ); \ ++ CHECK_FOR_LIVEPATCH_WORK \ ++ instr "1" \ ++ : : "r" (guest_cpu_user_regs()), constr (fn) : "memory" ); \ + unreachable(); \ + }) + ++#define reset_stack_and_jump(fn) \ ++ switch_stack_and_jump(fn, "jmp %c", "i") ++ ++/* The constraint may only specify non-call-clobbered registers. */ ++#define reset_stack_and_jump_ind(fn) \ ++ switch_stack_and_jump(fn, "INDIRECT_JMP %", "b") ++ + /* + * Which VCPU's state is currently running on each CPU? + * This is not necesasrily the same as 'current' as a CPU may be +diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h +index 360c38b..045a563 100644 +--- a/xen/include/asm-x86/domain.h ++++ b/xen/include/asm-x86/domain.h +@@ -328,7 +328,7 @@ struct arch_domain + const struct arch_csw { + void (*from)(struct vcpu *); + void (*to)(struct vcpu *); +- void (*tail)(struct vcpu *); ++ void noreturn (*tail)(void); + } *ctxt_switch; + + /* nestedhvm: translate l2 guest physical to host physical */ +diff --git a/xen/include/asm-x86/hvm/vmx/vmx.h b/xen/include/asm-x86/hvm/vmx/vmx.h +index 20eb7f6..36663bb 100644 +--- a/xen/include/asm-x86/hvm/vmx/vmx.h ++++ b/xen/include/asm-x86/hvm/vmx/vmx.h +@@ -95,7 +95,7 @@ typedef enum { + void vmx_asm_vmexit_handler(struct cpu_user_regs); + void vmx_asm_do_vmentry(void); + void vmx_intr_assist(void); +-void noreturn vmx_do_resume(struct vcpu *); ++void noreturn vmx_do_resume(void); + void vmx_vlapic_msr_changed(struct vcpu *v); + void vmx_realmode_emulate_one(struct hvm_emulate_ctxt *hvmemul_ctxt); + void vmx_realmode(struct cpu_user_regs *regs); diff -Nru xen-4.11.4+57-g41a822c392/debian/patches/0075-evtchn-FIFO-re-order-and-synchronize-with-map_contro.patch xen-4.11.4+57-g41a822c392/debian/patches/0075-evtchn-FIFO-re-order-and-synchronize-with-map_contro.patch --- xen-4.11.4+57-g41a822c392/debian/patches/0075-evtchn-FIFO-re-order-and-synchronize-with-map_contro.patch 1970-01-01 00:00:00.000000000 +0000 +++ xen-4.11.4+57-g41a822c392/debian/patches/0075-evtchn-FIFO-re-order-and-synchronize-with-map_contro.patch 2020-12-11 21:10:09.000000000 +0000 @@ -0,0 +1,59 @@ +From: Jan Beulich +Date: Fri, 11 Dec 2020 22:08:24 +0100 +Subject: evtchn/FIFO: re-order and synchronize (with) map_control_block() + +For evtchn_fifo_set_pending()'s check of the control block having been +set to be effective, ordering of respective reads and writes needs to be +ensured: The control block pointer needs to be recorded strictly after +the setting of all the queue heads, and it needs checking strictly +before any uses of them (this latter aspect was already guaranteed). + +This is XSA-358 / CVE-2020-29570. + +Signed-off-by: Jan Beulich +Acked-by: Julien Grall +--- + xen/common/event_fifo.c | 14 ++++++++++++-- + 1 file changed, 12 insertions(+), 2 deletions(-) + +diff --git a/xen/common/event_fifo.c b/xen/common/event_fifo.c +index 0a90a84..ab9e496 100644 +--- a/xen/common/event_fifo.c ++++ b/xen/common/event_fifo.c +@@ -227,6 +227,10 @@ static void evtchn_fifo_set_pending(struct vcpu *v, struct evtchn *evtchn) + goto unlock; + } + ++ /* ++ * This also acts as the read counterpart of the smp_wmb() in ++ * map_control_block(). ++ */ + if ( guest_test_and_set_bit(d, EVTCHN_FIFO_LINKED, word) ) + goto unlock; + +@@ -452,6 +456,7 @@ static int setup_control_block(struct vcpu *v) + static int map_control_block(struct vcpu *v, uint64_t gfn, uint32_t offset) + { + void *virt; ++ struct evtchn_fifo_control_block *control_block; + unsigned int i; + int rc; + +@@ -462,10 +467,15 @@ static int map_control_block(struct vcpu *v, uint64_t gfn, uint32_t offset) + if ( rc < 0 ) + return rc; + +- v->evtchn_fifo->control_block = virt + offset; ++ control_block = virt + offset; + + for ( i = 0; i <= EVTCHN_FIFO_PRIORITY_MIN; i++ ) +- v->evtchn_fifo->queue[i].head = &v->evtchn_fifo->control_block->head[i]; ++ v->evtchn_fifo->queue[i].head = &control_block->head[i]; ++ ++ /* All queue heads must have been set before setting the control block. */ ++ smp_wmb(); ++ ++ v->evtchn_fifo->control_block = control_block; + + return 0; + } diff -Nru xen-4.11.4+57-g41a822c392/debian/patches/0076-evtchn-FIFO-add-2nd-smp_rmb-to-evtchn_fifo_word_from.patch xen-4.11.4+57-g41a822c392/debian/patches/0076-evtchn-FIFO-add-2nd-smp_rmb-to-evtchn_fifo_word_from.patch --- xen-4.11.4+57-g41a822c392/debian/patches/0076-evtchn-FIFO-add-2nd-smp_rmb-to-evtchn_fifo_word_from.patch 1970-01-01 00:00:00.000000000 +0000 +++ xen-4.11.4+57-g41a822c392/debian/patches/0076-evtchn-FIFO-add-2nd-smp_rmb-to-evtchn_fifo_word_from.patch 2020-12-11 21:10:09.000000000 +0000 @@ -0,0 +1,45 @@ +From: Jan Beulich +Date: Fri, 11 Dec 2020 22:09:54 +0100 +Subject: evtchn/FIFO: add 2nd smp_rmb() to evtchn_fifo_word_from_port() + +Besides with add_page_to_event_array() the function also needs to +synchronize with evtchn_fifo_init_control() setting both d->evtchn_fifo +and (subsequently) d->evtchn_port_ops. + +This is XSA-359 / CVE-2020-29571. + +Signed-off-by: Jan Beulich +Reviewed-by: Julien Grall +--- + xen/common/event_fifo.c | 11 +++++++++++ + 1 file changed, 11 insertions(+) + +diff --git a/xen/common/event_fifo.c b/xen/common/event_fifo.c +index ab9e496..454ca40 100644 +--- a/xen/common/event_fifo.c ++++ b/xen/common/event_fifo.c +@@ -34,6 +34,13 @@ static inline event_word_t *evtchn_fifo_word_from_port(const struct domain *d, + { + unsigned int p, w; + ++ /* ++ * Callers aren't required to hold d->event_lock, so we need to synchronize ++ * with evtchn_fifo_init_control() setting d->evtchn_port_ops /after/ ++ * d->evtchn_fifo. ++ */ ++ smp_rmb(); ++ + if ( unlikely(port >= d->evtchn_fifo->num_evtchns) ) + return NULL; + +@@ -590,6 +597,10 @@ int evtchn_fifo_init_control(struct evtchn_init_control *init_control) + if ( rc < 0 ) + goto error; + ++ /* ++ * This call, as a side effect, synchronizes with ++ * evtchn_fifo_word_from_port(). ++ */ + rc = map_control_block(v, gfn, offset); + if ( rc < 0 ) + goto error; diff -Nru xen-4.11.4+57-g41a822c392/debian/patches/series xen-4.11.4+57-g41a822c392/debian/patches/series --- xen-4.11.4+57-g41a822c392/debian/patches/series 2020-12-03 12:56:29.000000000 +0000 +++ xen-4.11.4+57-g41a822c392/debian/patches/series 2020-12-11 21:10:09.000000000 +0000 @@ -47,3 +47,30 @@ 0047-pygrub-Set-sys.path.patch 0048-pygrub-Specify-rpath-LIBEXEC_LIB-when-building-fsima.patch 0049-tools-xl-bash-completion-also-complete-xen.patch +0050-tools-ocaml-xenstored-do-permission-checks-on-xensto.patch +0051-tools-xenstore-allow-removing-child-of-a-node-exceed.patch +0052-tools-xenstore-ignore-transaction-id-for-un-watch.patch +0053-tools-xenstore-fix-node-accounting-after-failed-node.patch +0054-tools-xenstore-simplify-and-rename-check_event_node.patch +0055-tools-xenstore-check-privilege-for-XS_IS_DOMAIN_INTR.patch +0056-tools-xenstore-rework-node-removal.patch +0057-tools-xenstore-fire-watches-only-when-removing-a-spe.patch +0058-tools-xenstore-introduce-node_perms-structure.patch +0059-tools-xenstore-allow-special-watches-for-privileged-.patch +0060-tools-xenstore-avoid-watch-events-for-nodes-without-.patch +0061-tools-ocaml-xenstored-ignore-transaction-id-for-un-w.patch +0062-tools-ocaml-xenstored-check-privilege-for-XS_IS_DOMA.patch +0063-tools-ocaml-xenstored-unify-watch-firing.patch +0064-tools-ocaml-xenstored-introduce-permissions-for-spec.patch +0065-tools-ocaml-xenstored-avoid-watch-events-for-nodes-w.patch +0066-tools-ocaml-xenstored-add-xenstored.conf-flag-to-tur.patch +0067-tools-xenstore-revoke-access-rights-for-removed-doma.patch +0068-tools-ocaml-xenstored-clean-up-permissions-for-dead-.patch +0069-tools-ocaml-xenstored-Fix-path-length-validation.patch +0070-tools-xenstore-drop-watch-event-messages-exceeding-m.patch +0071-tools-xenstore-Preserve-bad-client-until-they-are-de.patch +0072-tools-ocaml-xenstored-delete-watch-from-trie-too-whe.patch +0073-tools-ocaml-xenstored-only-Dom0-can-change-node-owne.patch +0074-x86-avoid-calling-svm-vmx-_do_resume.patch +0075-evtchn-FIFO-re-order-and-synchronize-with-map_contro.patch +0076-evtchn-FIFO-add-2nd-smp_rmb-to-evtchn_fifo_word_from.patch