Re: [Qemu-devel] [PATCH] qapi: change QmpInputVisitor to QSLIST

[Markus Armbruster]

Paolo Bonzini <address@hidden> writes:

> On 07/07/2016 10:19, Markus Armbruster wrote:
>> Actually, you should either prove that untrusted input still cannot make
>> us allocated unbounded amounts of memory, or bring the limit right back.
>
> This is not where untrusted input can be blocked from allocating
> unbounded memory---that would be QmpOutputVisitor, which converts a
> stream of visitor calls into a QObject.
>
> The QmpInputVisitor's allocation depth is bounded by the number of
> levels in the incoming QObject, so a QmpInputVisitor cannot allocate
> more memory than whatever has been allocated already by QEMU.
>
> In addition, QmpOutputVisitor allocates memory not just for the stack
> but also a QObject for every *value*.  So you can make QmpOutputVisitor
> allocate unbounded memory even with a single huge QDict.

A QAPI visit walks a (possibly degenerate, possibly conceptual) QAPI
tree, calling visitor methods along the way.

The QMP output visitor's methods build a QObject tree mirroring the QAPI
tree.  It is typically used to convert a real QAPI tree representing a
command response or event to a QObject tree that is then converted to
JSON and sent over the QMP wire.  In this usage, input is trusted.

Other uses exist[*].  They look safe to me.

The QMP input visitor's methods build a QAPI tree.  It is typically used
to convert a QObject tree we got from parsing the QMP wire into a QAPI
tree.  The text received on the QMP wire is untrusted input.  However,
the JSON parser already takes pains to limit the QObject tree it
creates.  Therefore, we don't need the QMP input visitor to limit it
again.

Related: dynamic conversions of QAPI 'any' values, like
user_creatable_add_type() does for qmp_object_add().  Same thing as a
generated QAPI visit, only at run time rather than compile time.  Also
protected by the JSON parser.  Likewise, object_property_set_qobject()
for qmp_qom_set().

object_property_set_str() & friends are obviously safe, because they
only pass it only scalars they create themselves.

This is the kind of proof I wanted to see.  Looking forward to your
rebased v3.


[*] A funny one is qmp_blockdev_add(), which uses the QMP output visitor
to convert the QAPI tree we laboriously constructed from JSON via
QObject right back to QObject.  Oh well.