diff options
Diffstat (limited to '0098-tools-xenstore-add-memory-accounting-for-nodes.patch')
| -rw-r--r-- | 0098-tools-xenstore-add-memory-accounting-for-nodes.patch | 342 |
1 files changed, 342 insertions, 0 deletions
diff --git a/0098-tools-xenstore-add-memory-accounting-for-nodes.patch b/0098-tools-xenstore-add-memory-accounting-for-nodes.patch new file mode 100644 index 0000000..f2f8e4f --- /dev/null +++ b/0098-tools-xenstore-add-memory-accounting-for-nodes.patch @@ -0,0 +1,342 @@ +From 32efe29a00efab2896cc973e966a35ecad556495 Mon Sep 17 00:00:00 2001 +From: Juergen Gross <jgross@suse.com> +Date: Tue, 13 Sep 2022 07:35:10 +0200 +Subject: [PATCH 098/126] tools/xenstore: add memory accounting for nodes + +Add the memory accounting for Xenstore nodes. In order to make this +not too complicated allow for some sloppiness when writing nodes. Any +hard quota violation will result in no further requests to be accepted. + +This is part of XSA-326 / CVE-2022-42315. + +Signed-off-by: Juergen Gross <jgross@suse.com> +Reviewed-by: Julien Grall <jgrall@amazon.com> +(cherry picked from commit 00e9e32d022be1afc144b75acdaeba8393e63315) +--- + tools/xenstore/xenstored_core.c | 140 ++++++++++++++++++++++--- + tools/xenstore/xenstored_core.h | 12 +++ + tools/xenstore/xenstored_transaction.c | 16 ++- + 3 files changed, 151 insertions(+), 17 deletions(-) + +diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c +index b1a4575929bd..f27d5c0101bc 100644 +--- a/tools/xenstore/xenstored_core.c ++++ b/tools/xenstore/xenstored_core.c +@@ -556,6 +556,117 @@ void set_tdb_key(const char *name, TDB_DATA *key) + key->dsize = strlen(name); + } + ++static void get_acc_data(TDB_DATA *key, struct node_account_data *acc) ++{ ++ TDB_DATA old_data; ++ struct xs_tdb_record_hdr *hdr; ++ ++ if (acc->memory < 0) { ++ old_data = tdb_fetch(tdb_ctx, *key); ++ /* No check for error, as the node might not exist. */ ++ if (old_data.dptr == NULL) { ++ acc->memory = 0; ++ } else { ++ hdr = (void *)old_data.dptr; ++ acc->memory = old_data.dsize; ++ acc->domid = hdr->perms[0].id; ++ } ++ talloc_free(old_data.dptr); ++ } ++} ++ ++/* ++ * Per-transaction nodes need to be accounted for the transaction owner. ++ * Those nodes are stored in the data base with the transaction generation ++ * count prepended (e.g. 123/local/domain/...). So testing for the node's ++ * key not to start with "/" is sufficient. ++ */ ++static unsigned int get_acc_domid(struct connection *conn, TDB_DATA *key, ++ unsigned int domid) ++{ ++ return (!conn || key->dptr[0] == '/') ? domid : conn->id; ++} ++ ++int do_tdb_write(struct connection *conn, TDB_DATA *key, TDB_DATA *data, ++ struct node_account_data *acc, bool no_quota_check) ++{ ++ struct xs_tdb_record_hdr *hdr = (void *)data->dptr; ++ struct node_account_data old_acc = {}; ++ unsigned int old_domid, new_domid; ++ int ret; ++ ++ if (!acc) ++ old_acc.memory = -1; ++ else ++ old_acc = *acc; ++ ++ get_acc_data(key, &old_acc); ++ old_domid = get_acc_domid(conn, key, old_acc.domid); ++ new_domid = get_acc_domid(conn, key, hdr->perms[0].id); ++ ++ /* ++ * Don't check for ENOENT, as we want to be able to switch orphaned ++ * nodes to new owners. ++ */ ++ if (old_acc.memory) ++ domain_memory_add_nochk(old_domid, ++ -old_acc.memory - key->dsize); ++ ret = domain_memory_add(new_domid, data->dsize + key->dsize, ++ no_quota_check); ++ if (ret) { ++ /* Error path, so no quota check. */ ++ if (old_acc.memory) ++ domain_memory_add_nochk(old_domid, ++ old_acc.memory + key->dsize); ++ return ret; ++ } ++ ++ /* TDB should set errno, but doesn't even set ecode AFAICT. */ ++ if (tdb_store(tdb_ctx, *key, *data, TDB_REPLACE) != 0) { ++ domain_memory_add_nochk(new_domid, -data->dsize - key->dsize); ++ /* Error path, so no quota check. */ ++ if (old_acc.memory) ++ domain_memory_add_nochk(old_domid, ++ old_acc.memory + key->dsize); ++ errno = EIO; ++ return errno; ++ } ++ ++ if (acc) { ++ /* Don't use new_domid, as it might be a transaction node. */ ++ acc->domid = hdr->perms[0].id; ++ acc->memory = data->dsize; ++ } ++ ++ return 0; ++} ++ ++int do_tdb_delete(struct connection *conn, TDB_DATA *key, ++ struct node_account_data *acc) ++{ ++ struct node_account_data tmp_acc; ++ unsigned int domid; ++ ++ if (!acc) { ++ acc = &tmp_acc; ++ acc->memory = -1; ++ } ++ ++ get_acc_data(key, acc); ++ ++ if (tdb_delete(tdb_ctx, *key)) { ++ errno = EIO; ++ return errno; ++ } ++ ++ if (acc->memory) { ++ domid = get_acc_domid(conn, key, acc->domid); ++ domain_memory_add_nochk(domid, -acc->memory - key->dsize); ++ } ++ ++ return 0; ++} ++ + /* + * If it fails, returns NULL and sets errno. + * Temporary memory allocations will be done with ctx. +@@ -609,9 +720,15 @@ struct node *read_node(struct connection *conn, const void *ctx, + + /* Permissions are struct xs_permissions. */ + node->perms.p = hdr->perms; ++ node->acc.domid = node->perms.p[0].id; ++ node->acc.memory = data.dsize; + if (domain_adjust_node_perms(conn, node)) + goto error; + ++ /* If owner is gone reset currently accounted memory size. */ ++ if (node->acc.domid != node->perms.p[0].id) ++ node->acc.memory = 0; ++ + /* Data is binary blob (usually ascii, no nul). */ + node->data = node->perms.p + hdr->num_perms; + /* Children is strings, nul separated. */ +@@ -680,12 +797,9 @@ int write_node_raw(struct connection *conn, TDB_DATA *key, struct node *node, + p += node->datalen; + memcpy(p, node->children, node->childlen); + +- /* TDB should set errno, but doesn't even set ecode AFAICT. */ +- if (tdb_store(tdb_ctx, *key, data, TDB_REPLACE) != 0) { +- corrupt(conn, "Write of %s failed", key->dptr); +- errno = EIO; +- return errno; +- } ++ if (do_tdb_write(conn, key, &data, &node->acc, no_quota_check)) ++ return EIO; ++ + return 0; + } + +@@ -1188,7 +1302,7 @@ static void delete_node_single(struct connection *conn, struct node *node) + if (access_node(conn, node, NODE_ACCESS_DELETE, &key)) + return; + +- if (tdb_delete(tdb_ctx, key) != 0) { ++ if (do_tdb_delete(conn, &key, &node->acc) != 0) { + corrupt(conn, "Could not delete '%s'", node->name); + return; + } +@@ -1261,6 +1375,7 @@ static struct node *construct_node(struct connection *conn, const void *ctx, + /* No children, no data */ + node->children = node->data = NULL; + node->childlen = node->datalen = 0; ++ node->acc.memory = 0; + node->parent = parent; + return node; + +@@ -1269,17 +1384,17 @@ nomem: + return NULL; + } + +-static void destroy_node_rm(struct node *node) ++static void destroy_node_rm(struct connection *conn, struct node *node) + { + if (streq(node->name, "/")) + corrupt(NULL, "Destroying root node!"); + +- tdb_delete(tdb_ctx, node->key); ++ do_tdb_delete(conn, &node->key, &node->acc); + } + + static int destroy_node(struct connection *conn, struct node *node) + { +- destroy_node_rm(node); ++ destroy_node_rm(conn, node); + domain_entry_dec(conn, node); + + /* +@@ -1331,7 +1446,7 @@ static struct node *create_node(struct connection *conn, const void *ctx, + /* Account for new node */ + if (i->parent) { + if (domain_entry_inc(conn, i)) { +- destroy_node_rm(i); ++ destroy_node_rm(conn, i); + return NULL; + } + } +@@ -2192,7 +2307,7 @@ static int clean_store_(TDB_CONTEXT *tdb, TDB_DATA key, TDB_DATA val, + if (!hashtable_search(reachable, name)) { + log("clean_store: '%s' is orphaned!", name); + if (recovery) { +- tdb_delete(tdb, key); ++ do_tdb_delete(NULL, &key, NULL); + } + } + +@@ -3030,6 +3145,7 @@ void read_state_node(const void *ctx, const void *state) + if (!node) + barf("allocation error restoring node"); + ++ node->acc.memory = 0; + node->name = name; + node->generation = ++generation; + node->datalen = sn->data_len; +diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h +index 2fb37dbfe847..5c1b574bffe6 100644 +--- a/tools/xenstore/xenstored_core.h ++++ b/tools/xenstore/xenstored_core.h +@@ -169,6 +169,11 @@ struct node_perms { + struct xs_permissions *p; + }; + ++struct node_account_data { ++ unsigned int domid; ++ int memory; /* -1 if unknown */ ++}; ++ + struct node { + const char *name; + /* Key used to update TDB */ +@@ -191,6 +196,9 @@ struct node { + /* Children, each nul-terminated. */ + unsigned int childlen; + char *children; ++ ++ /* Allocation information for node currently in store. */ ++ struct node_account_data acc; + }; + + /* Return the only argument in the input. */ +@@ -300,6 +308,10 @@ extern xengnttab_handle **xgt_handle; + int remember_string(struct hashtable *hash, const char *str); + + void set_tdb_key(const char *name, TDB_DATA *key); ++int do_tdb_write(struct connection *conn, TDB_DATA *key, TDB_DATA *data, ++ struct node_account_data *acc, bool no_quota_check); ++int do_tdb_delete(struct connection *conn, TDB_DATA *key, ++ struct node_account_data *acc); + + void conn_free_buffered_data(struct connection *conn); + +diff --git a/tools/xenstore/xenstored_transaction.c b/tools/xenstore/xenstored_transaction.c +index 7bd41eb475e3..ace9a11d77bb 100644 +--- a/tools/xenstore/xenstored_transaction.c ++++ b/tools/xenstore/xenstored_transaction.c +@@ -153,6 +153,9 @@ struct transaction + /* List of all transactions active on this connection. */ + struct list_head list; + ++ /* Connection this transaction is associated with. */ ++ struct connection *conn; ++ + /* Connection-local identifier for this transaction. */ + uint32_t id; + +@@ -286,6 +289,8 @@ int access_node(struct connection *conn, struct node *node, + + introduce = true; + i->ta_node = false; ++ /* acc.memory < 0 means "unknown, get size from TDB". */ ++ node->acc.memory = -1; + + /* + * Additional transaction-specific node for read type. We only +@@ -410,11 +415,11 @@ static int finalize_transaction(struct connection *conn, + goto err; + hdr = (void *)data.dptr; + hdr->generation = ++generation; +- ret = tdb_store(tdb_ctx, key, data, +- TDB_REPLACE); ++ ret = do_tdb_write(conn, &key, &data, NULL, ++ true); + talloc_free(data.dptr); + } else { +- ret = tdb_delete(tdb_ctx, key); ++ ret = do_tdb_delete(conn, &key, NULL); + } + if (ret) + goto err; +@@ -425,7 +430,7 @@ static int finalize_transaction(struct connection *conn, + } + } + +- if (i->ta_node && tdb_delete(tdb_ctx, ta_key)) ++ if (i->ta_node && do_tdb_delete(conn, &ta_key, NULL)) + goto err; + list_del(&i->list); + talloc_free(i); +@@ -453,7 +458,7 @@ static int destroy_transaction(void *_transaction) + i->node); + if (trans_name) { + set_tdb_key(trans_name, &key); +- tdb_delete(tdb_ctx, key); ++ do_tdb_delete(trans->conn, &key, NULL); + } + } + list_del(&i->list); +@@ -497,6 +502,7 @@ int do_transaction_start(struct connection *conn, struct buffered_data *in) + + INIT_LIST_HEAD(&trans->accessed); + INIT_LIST_HEAD(&trans->changed_domains); ++ trans->conn = conn; + trans->fail = false; + trans->generation = ++generation; + +-- +2.37.4 + |