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

[Juergen Gross <jgross@xxxxxxxx>]

On 20.02.23 10:46, Julien Grall wrote:
> Hi Juergen,
>
> On 20/01/2023 10: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.
> I know I said I would delay my decision on this patch. However, I was still 
> expecting the commit message to be updated based on our previous discussion.
As said before, I was waiting on the settlement of our discussion before
doing the update.

> Also thinking more about it, "The transaction will finally fail due to exceeding 
> the number of nodes quota" may not be true for a couple of reasons:
>    1) The transaction may removed a node afterwards.
Yes, and? Just because it is a transaction, this is still a violation of
the quota. Even outside a transaction you could use the same reasoning,
but you don't (which is correct, of course).

In case you never finish the transaction, you are owner of more than
allowed nodes.

>    2) A node may have been removed outside of the transaction.
If the removed node hasn't been touched by the transaction, it will be
accounted for correctly. If it has been touched, the transaction will
fail anyway.

> In both situation, the transaction will still be committed. This will now be 
> prevented by this patch.
As said above, only in the first case.

> While I understand, they may be edge cases, this is also true for what you are 
> aiming to solve. So I am still not convinced about the benefits of this patch.

Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature