On 06.11.22 23:00, Julien Grall wrote:
Hi Juergen,

On 01/11/2022 15:28, 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 have already made some of comments on the security ML when this was initially set. The arguments still don't sound right to me.

For a first, since XSA-326, we have a cap in place for the memory a domain can consume. So this surely can't be a "lot of memory". Otherwise we are saying that our limit (there are other way to hit it) were wrong...

Yeah, maybe I should rework the commit message.

The reason I still want to keep the patch is that in a transaction without any
conflicts and without hitting any transaction specific limits (number of nodes
accessed), the errors returned due to a single operation should be the same as
with the same operation performed outside a transaction.

In addition to that, this is quite pointless to check the number of entry a domain currently owned because this can change before the transaction is committed by another "external" command. Therefore, this would add some randomness in the command which could make more difficult to diagnose.

In the scope of the transaction the tests are correct.

Lastly, there are other very easy way to use memory (just create multiple transaction in parallel).

So based on the commit message, the change doesn't help at all to make better Xenstored.

Note that I don't particularly mind the code change (even though it adds more complexity). I just strongly dislike the current justifications.

At the moment, I can't find a justification that would make the change 
whorthwhile.

See above reasoning.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature

Reply via email to