On Freitag, 28. Januar 2022 12:49:58 CET Christian Schoenebeck wrote:
> On Mittwoch, 26. Januar 2022 18:11:36 CET Greg Kurz wrote:
> > The template pointer in virtio_9p_create_local_test_dir() is leaked.
> > Add the g_autofree annotation to fix that. While here, convert the
> > rest of the virtio 9p test code to using g_autofree or g_autoptr
> > where possible, since this is the preferred approach to avoid potential
> > leaks in the future.
> >
> > Based-on:
> > <f6602123c6f7d0d593466231b04fba087817abbd.1642879848.git.qemu_oss@crudebyt
> > e
> > .com> Signed-off-by: Greg Kurz <gr...@kaod.org>
> > ---
> >
> > tests/qtest/libqos/virtio-9p.c | 15 +++++----------
> > 1 file changed, 5 insertions(+), 10 deletions(-)
>
> I fear there is something wrong with this patch:
>
> # Start of local tests
> # starting QEMU: exec x86_64-softmmu/qemu-system-x86_64 -qtest
> unix:/tmp/qtest-4234.sock -qtest-log /dev/null -chardev
> socket,path=/tmp/qtest-4234.qmp,id=char0 -mon chardev=char0,mode=control
> -display none -M pc -fsdev
> local,id=fsdev0,path='',security_model=mapped-xattr -device
> virtio-9p-pci,fsdev=fsdev0,addr=04.0,mount_tag=qtest -accel qtest
> qemu-system-x86_64: -device
> virtio-9p-pci,fsdev=fsdev0,addr=04.0,mount_tag=qtest: cannot initialize
> fsdev 'fsdev0': failed to open '': No such file or directory Broken pipe
> Aborted
Reason ...
> > diff --git a/tests/qtest/libqos/virtio-9p.c
> > b/tests/qtest/libqos/virtio-9p.c index ef96ef006adc..0a0d0d16709b 100644
> > --- a/tests/qtest/libqos/virtio-9p.c
> > +++ b/tests/qtest/libqos/virtio-9p.c
> > @@ -40,14 +40,13 @@ static char *concat_path(const char* a, const char* b)
> >
> > void virtio_9p_create_local_test_dir(void)
> > {
> >
> > struct stat st;
> >
> > - char *pwd = g_get_current_dir();
> > - char *template = concat_path(pwd, "qtest-9p-local-XXXXXX");
> > + g_autofree char *pwd = g_get_current_dir();
> > + g_autofree char *template = concat_path(pwd,
> > "qtest-9p-local-XXXXXX");
> >
> > local_test_path = mkdtemp(template);
... mkdtemp() does not allocate a new buffer, it just modifies the character
array passed, i.e. the address returned by mkdtemp() equals the address of
variable 'template', and when virtio_9p_create_local_test_dir() scope is left,
the global variable 'local_test_path' would then point to freed memory.
I would drop g_autofree from template:
char *template = concat_path(pwd, "qtest-9p-local-XXXXXX");
And if it helps to silence a leak warning (haven't tested), to prepend
g_autofree to the global variable instead:
static g_autofree char *local_test_path;
Best regards,
Christian Schoenebeck
