On 24.04.23 10:56, Jan Beulich wrote:
On 28.03.2023 16:43, Juergen Gross wrote:Today when finalizing a transaction the number of node quota is checked to not being exceeded after the transaction. This check is always done, even if the transaction is being performed by a privileged connection, or if there were no nodes created in the transaction.Correct that by checking quota only if: - the transaction is being performed by an unprivileged guest, and - at least one node was created in the transaction Reported-by: Julien Grall <[email protected]> Fixes: f2bebf72c4d5 ("xenstore: rework of transaction handling") Signed-off-by: Juergen Gross <[email protected]>Considering the referenced commit is about 6 years old, I thought this would be a backporting candidate. The function mentioned in the title doesn't appear to exist on 4.17, though. Therefore, if there is an intention for this to be backported, may I please ask that a suitable backport be provided?
This should do the job. Juergen
From 1e308552989dd3c371d5ff27d8e615d07ca9847d Mon Sep 17 00:00:00 2001 From: Juergen Gross <[email protected]> Date: Mon, 24 Apr 2023 11:41:26 +0200 Subject: [PATCH] tools/xenstore: fix quota check in acc_fix_domains() Today when finalizing a transaction the number of node quota is checked to not being exceeded after the transaction. This check is always done, even if the transaction is being performed by a privileged connection, or if there were no nodes created in the transaction. Correct that by checking quota only if: - the transaction is being performed by an unprivileged guest, and - at least one node was created in the transaction Commit f6b801c36bd5e4ab22a9f80c8d57121b62b139af upstream. Reported-by: Julien Grall <[email protected]> Fixes: f2bebf72c4d5 ("xenstore: rework of transaction handling") Signed-off-by: Juergen Gross <[email protected]> --- tools/xenstore/xenstored_core.c | 3 +++ tools/xenstore/xenstored_transaction.c | 24 ++++++++++++++++++++---- tools/xenstore/xenstored_transaction.h | 3 +++ 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c index 56dbdc2530..8907c6cb0b 100644 --- a/tools/xenstore/xenstored_core.c +++ b/tools/xenstore/xenstored_core.c @@ -1474,6 +1474,9 @@ static struct node *create_node(struct connection *conn, const void *ctx, if (!node) return NULL; + if (conn && conn->transaction) + ta_node_created(conn->transaction); + node->data = data; node->datalen = datalen; diff --git a/tools/xenstore/xenstored_transaction.c b/tools/xenstore/xenstored_transaction.c index ac854197ca..8ebd16aed2 100644 --- a/tools/xenstore/xenstored_transaction.c +++ b/tools/xenstore/xenstored_transaction.c @@ -172,12 +172,20 @@ struct transaction /* List of changed domains - to record the changed domain entry number */ struct list_head changed_domains; + /* There was at least one node created in the transaction. */ + bool node_created; + /* Flag for letting transaction fail. */ bool fail; }; uint64_t generation; +void ta_node_created(struct transaction *trans) +{ + trans->node_created = true; +} + static struct accessed_node *find_accessed_node(struct transaction *trans, const char *name) { @@ -514,7 +522,12 @@ int do_transaction_start(const void *ctx, struct connection *conn, return 0; } -static int transaction_fix_domains(struct transaction *trans, bool update) +/* + * Update or check number of nodes per domain at the end of a transaction. + * If "update" is true, "chk_quota" is ignored. + */ +static int transaction_fix_domains(struct transaction *trans, bool chk_quota, + bool update) { struct changed_domain *d; int cnt; @@ -522,7 +535,7 @@ static int transaction_fix_domains(struct transaction *trans, bool update) list_for_each_entry(d, &trans->changed_domains, list) { cnt = domain_entry_fix(d->domid, d->nbentry, update); if (!update) { - if (cnt >= quota_nb_entry_per_domain) + if (chk_quota && cnt >= quota_nb_entry_per_domain) return ENOSPC; if (cnt < 0) return ENOMEM; @@ -538,6 +551,7 @@ int do_transaction_end(const void *ctx, struct connection *conn, const char *arg = onearg(in); struct transaction *trans; bool is_corrupt = false; + bool chk_quota; int ret; if (!arg || (!streq(arg, "T") && !streq(arg, "F"))) @@ -552,13 +566,15 @@ int do_transaction_end(const void *ctx, struct connection *conn, if (!conn->transaction_started) conn->ta_start_time = 0; + chk_quota = trans->node_created && domain_is_unprivileged(conn); + /* Attach transaction to ctx for auto-cleanup */ talloc_steal(ctx, trans); if (streq(arg, "T")) { if (trans->fail) return ENOMEM; - ret = transaction_fix_domains(trans, false); + ret = transaction_fix_domains(trans, chk_quota, false); if (ret) return ret; ret = finalize_transaction(conn, trans, &is_corrupt); @@ -568,7 +584,7 @@ int do_transaction_end(const void *ctx, struct connection *conn, wrl_apply_debit_trans_commit(conn); /* fix domain entry for each changed domain */ - transaction_fix_domains(trans, true); + transaction_fix_domains(trans, false, true); if (is_corrupt) corrupt(conn, "transaction inconsistency"); diff --git a/tools/xenstore/xenstored_transaction.h b/tools/xenstore/xenstored_transaction.h index 3417303f94..f57d204cc0 100644 --- a/tools/xenstore/xenstored_transaction.h +++ b/tools/xenstore/xenstored_transaction.h @@ -36,6 +36,9 @@ int do_transaction_end(const void *ctx, struct connection *conn, struct transaction *transaction_lookup(struct connection *conn, uint32_t id); +/* Set flag for created node. */ +void ta_node_created(struct transaction *trans); + /* inc/dec entry number local to trans while changing a node */ void transaction_entry_inc(struct transaction *trans, unsigned int domid); void transaction_entry_dec(struct transaction *trans, unsigned int domid); -- 2.35.3
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature
Description: OpenPGP digital signature
