Incorrect error handling in xen_enable_tpm()

[Markus Armbruster <armbru@xxxxxxxxxx>]

Consider xen_enable_tpm() in hw/xen/xen-pvh-common.c:

    static void xen_enable_tpm(XenPVHMachineState *s)
    {
        Error *errp = NULL;

Nitpick: confusing name; we use @errp for Error ** variables, not for
Error * variables.  Better: @err.

        DeviceState *dev;
        SysBusDevice *busdev;

        TPMBackend *be = qemu_find_tpm_be("tpm0");
        if (be == NULL) {
            error_report("Couldn't find tmp0 backend");
            return;
        }
        dev = qdev_new(TYPE_TPM_TIS_SYSBUS);
        object_property_set_link(OBJECT(dev), "tpmdev", OBJECT(be), &errp);

If this fails, @errp changes to non-null.

        object_property_set_str(OBJECT(dev), "tpmdev", be->id, &errp);

If this then succeeds, @errp remains non-null.  Since we ignore it, we
leak the Error object.

If it also fails, we trip error_setv()'s assertion, because we violate
qapi/error.h rule

 * - You may pass NULL to not receive the error, &error_abort to abort
 *   on error, &error_fatal to exit(1) on error, or a pointer to a
 *   variable containing NULL to receive the error.

Taking a step back from the incorrect error handling: we're setting the
same property *twice*.  Why?

As far as I can tell, property "tpmdev" is not a link.  It's defined
with DEFINE_PROP_TPMBE(), i.e. setting it runs set_tpm() from
backends/tpm/tpm_util.c.  Passes the value to qemu_find_tpm_be(), which
appears to take a TPMBackend ID.

I suspect the object_property_set_link() always fails, and we leak the
Error object.

I'd verify this in a debugger, but I don't have a Xen box handy.

Code goes back to commit 733766cd373 (hw/arm: introduce xenpvh machine).

        busdev = SYS_BUS_DEVICE(dev);
        sysbus_realize_and_unref(busdev, &error_fatal);
        sysbus_mmio_map(busdev, 0, s->cfg.tpm.base);

        trace_xen_enable_tpm(s->cfg.tpm.base);
    }