[v2] tools/xenstore: do some cleanup and fixes

[PATCH v2 11/19] tools/xenstore: don't allow creating too many nodes in a transaction

Posted by Juergen Gross 1 year, 11 months ago

The accounting for the number of nodes of a domain in an active
transaction is not working correctly, as it allows to create arbitrary
number of nodes. The transaction will finally fail due to exceeding
the number of nodes quota, but before closing the transaction an
unprivileged guest could cause Xenstore to use a lot of memory.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/xenstore/xenstored_domain.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c
index 40b24056c5..1ae79b5b54 100644
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -1118,9 +1118,8 @@ int domain_nbentry_fix(unsigned int domid, int num, bool update)
 
 int domain_nbentry(struct connection *conn)
 {
-	return (domain_is_unprivileged(conn))
-		? conn->domain->nbentry
-		: 0;
+	return domain_is_unprivileged(conn)
+	       ? domain_nbentry_add(conn, conn->id, 0, true) : 0;
 }
 
 static bool domain_chk_quota(struct domain *domain, int mem)
-- 
2.35.3

Re: [PATCH v2 11/19] tools/xenstore: don't allow creating too many nodes in a transaction

Posted by Julien Grall 1 year, 11 months ago

Hi,

On 13/12/2022 16:00, Juergen Gross wrote:
> The accounting for the number of nodes of a domain in an active
> transaction is not working correctly, as it allows to create arbitrary
> number of nodes. The transaction will finally fail due to exceeding
> the number of nodes quota, but before closing the transaction an
> unprivileged guest could cause Xenstore to use a lot of memory.

As per the discussion in v1, the commit message needs to be reworded.

I will look at this patch in more details once I have reached the 2nd 
series.

Cheers,

-- 
Julien Grall

Re: [PATCH v2 11/19] tools/xenstore: don't allow creating too many nodes in a transaction

Posted by Juergen Gross 1 year, 10 months ago

On 20.12.22 21:18, Julien Grall wrote:
> Hi,
> 
> On 13/12/2022 16:00, Juergen Gross wrote:
>> The accounting for the number of nodes of a domain in an active
>> transaction is not working correctly, as it allows to create arbitrary
>> number of nodes. The transaction will finally fail due to exceeding
>> the number of nodes quota, but before closing the transaction an
>> unprivileged guest could cause Xenstore to use a lot of memory.
> 
> As per the discussion in v1, the commit message needs to be reworded.
> 
> I will look at this patch in more details once I have reached the 2nd series.

I'll wait with the rewording until then.


Juergen