| Message ID | 20240822162127.705879-3-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm: xlnx: fix minor memory leaks | expand |
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 > >
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)
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> --- hw/misc/xlnx-versal-trng.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)