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

[Julien Grall <julien@xxxxxxx>]

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...
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.
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.
Cheers,

--
Julien Grall