Re: [PATCH for-9.2 2/6] hw/misc/xlnx-versal-trng: Free s->prng in finalize, not unrealize

[Alistair Francis]

On Fri, Aug 23, 2024 at 2:22 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> The TYPE_XLNX_VERSAL_TRNG device creates s->prng with g_rand_new()
> in its init method, but it frees it in its unrealize method. This
> results in a leak in the QOM introspection "initialize-inspect-finalize"
> lifecycle:
>
> Direct leak of 2500 byte(s) in 1 object(s) allocated from:
>     #0 0x55ec89eae9d8 in __interceptor_calloc 
> (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/qemu-system-aarch64+0x294d9d8)
>  (BuildId: 6d5
> 08874816cc47d17c8dd775e8f809ae520e8cb)
>     #1 0x7f697018fc50 in g_malloc0 
> debian/build/deb/../../../glib/gmem.c:161:13
>     #2 0x7f6970197738 in g_rand_new_with_seed_array 
> debian/build/deb/../../../glib/grand.c:202:17
>     #3 0x7f6970197816 in g_rand_new 
> debian/build/deb/../../../glib/grand.c:286:10
>     #4 0x55ec8aa3656a in trng_init hw/misc/xlnx-versal-trng.c:624:15
>     #5 0x55ec8ce75da1 in object_init_with_type qom/object.c:420:9
>     #6 0x55ec8ce5d07b in object_initialize_with_type qom/object.c:562:5
>     #7 0x55ec8ce5e91d in object_new_with_type qom/object.c:782:5
>     #8 0x55ec8ce5e9f1 in object_new qom/object.c:797:12
>     #9 0x55ec8d65c81d in qmp_device_list_properties qom/qom-qmp-cmds.c:144:11
>
> Move the free to finalize so it matches where we are initing
> s->prng. Since that's the only thing our unrealize method was
> doing, this essentially switches the whole function to be
> a finalize implementation.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/misc/xlnx-versal-trng.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/misc/xlnx-versal-trng.c b/hw/misc/xlnx-versal-trng.c
> index 51eb7600414..c0d1dde8708 100644
> --- a/hw/misc/xlnx-versal-trng.c
> +++ b/hw/misc/xlnx-versal-trng.c
> @@ -624,9 +624,9 @@ static void trng_init(Object *obj)
>      s->prng = g_rand_new();
>  }
>
> -static void trng_unrealize(DeviceState *dev)
> +static void trng_finalize(Object *obj)
>  {
> -    XlnxVersalTRng *s = XLNX_VERSAL_TRNG(dev);
> +    XlnxVersalTRng *s = XLNX_VERSAL_TRNG(obj);
>
>      g_rand_free(s->prng);
>      s->prng = NULL;
> @@ -689,7 +689,6 @@ static void trng_class_init(ObjectClass *klass, void 
> *data)
>      ResettableClass *rc = RESETTABLE_CLASS(klass);
>
>      dc->vmsd = &vmstate_trng;
> -    dc->unrealize = trng_unrealize;
>      rc->phases.hold = trng_reset_hold;
>
>      /* Clone uint64 property with set allowed after realized */
> @@ -706,6 +705,7 @@ static const TypeInfo trng_info = {
>      .instance_size = sizeof(XlnxVersalTRng),
>      .class_init    = trng_class_init,
>      .instance_init = trng_init,
> +    .instance_finalize = trng_finalize,
>  };
>
>  static void trng_register_types(void)
> --
> 2.34.1
>
>