On Wed, Aug 26, 2020 at 09:16:48PM -0500, Eric Blake wrote: > Implementing .default_export with its 'const char *' return is tricky > in the sh plugin: we must return dynamic memory, but must avoid a > use-after-free. And we don't want to change the return type of > .default_export to 'char *', because that would make our choice of > malloc()/free() part of the API, preventing either nbdkit or a plugin > from experimenting with an alternate allocator implementation. > Elsewhere, we have done things like nbdkit_extents_new(), so that even > though the client is directing allocation, the actual call to malloc() > is done by nbdkit proper, so that nbdkit later calling free() does not > tie the plugin's hands, and nbdkit could change its underlying > allocation without breaking ABI. (Note that nbdkit_realpath() and > nbdkit_absolute_path() require the caller to use free(), but as they > are used during .config, it's less of a burden for plugins to take > care of that in .unload.) > > We could document that the burden is on the plugin to avoid the memory > leak, by making sh track all strings it returns, then finally clean > them up during .unload. But this is still a memory leak in the case > of .default_export, which can be called multiple times when there are > multiple clients, and presumably with different values per client > call; better would be freeing the memory at .close when each client > goes away. Except that .default_export is called before .open, and > therefore before we have access to the handle struct that will > eventually make it to .close. > > So, the solution is to let nbdkit track an alternate string on our > behalf, where nbdkit then cleans it up as the client goes away. > > There are several places in the existing code base that can also take > advantage of this copying functionality, including in filters that > want to pass altered strings to .config while still obeying lifetime > rules.
We should probably just push this one straight away, with one small change: > diff --git a/server/public.c b/server/public.c > index d10d466e..512e9caa 100644 > --- a/server/public.c > +++ b/server/public.c This file is probably going to need: #include "strndup.h" somewhere near the top because unbelievably Win32 lacks this function. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html _______________________________________________ Libguestfs mailing list [email protected] https://www.redhat.com/mailman/listinfo/libguestfs
