On Fri, Sep 30, 2022 at 10:42:01AM -0500, Eric Blake wrote: > Modern GCC has two related attributes for functions returning a > pointer: > > __attribute__((__malloc__)) - this function returns a new pointer, not > aliased to any existing pointer > > __attribute__((__malloc__(fn,1))) - call fn(return_value) to avoid > leaking memory allocated by this function > > With those attributes, static analyzers can better detect when we pass > the resulting pointer to the wrong deallocator, deallocate more than > once, have a use after free, or otherwise leak the memory. (Sadly, as > of gcc 12.2.1, -fanalyzer still has a LOT of false positives, such as: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107100; since our code > base triggers some of these, we can't quite rely on it yet). > --- > lib/internal.h | 4 +++- > generator/C.ml | 24 +++++++++++++++++++++--- > 2 files changed, 24 insertions(+), 4 deletions(-) > > diff --git a/lib/internal.h b/lib/internal.h > index 0e17fbea..22f500b4 100644 > --- a/lib/internal.h > +++ b/lib/internal.h > @@ -508,8 +508,10 @@ extern void nbd_internal_fork_safe_perror (const char *s) > extern char *nbd_internal_printable_buffer (const void *buf, size_t count) > LIBNBD_ATTRIBUTE_NONNULL(1); > extern char *nbd_internal_printable_string (const char *str) > + LIBNBD_ATTRIBUTE_ALLOC_DEALLOC(free) > LIBNBD_ATTRIBUTE_NONNULL(1); > -extern char *nbd_internal_printable_string_list (char **list); > +extern char *nbd_internal_printable_string_list (char **list) > + LIBNBD_ATTRIBUTE_ALLOC_DEALLOC(free); > > /* These are wrappers around socket(2) and socketpair(2). They > * always set SOCK_CLOEXEC. nbd_internal_socket can set SOCK_NONBLOCK > diff --git a/generator/C.ml b/generator/C.ml > index c74fae77..cfa2964c 100644 > --- a/generator/C.ml > +++ b/generator/C.ml > @@ -244,6 +244,16 @@ let > pr "extern "; > print_call ?wrap ?closure_style name args optargs ret; > > + (* For RString, note that the caller must free() the argument. > + * Since this is used in a public header, we must use gcc's spelling > + * of __builtin_free in case bare free is defined as a macro. > + *) > + (match ret with > + | RString -> > + pr "\n LIBNBD_ATTRIBUTE_ALLOC_DEALLOC(__builtin_free)"; > + | _ -> () > + ); > + > (* Output __attribute__((nonnull)) for the function parameters: > * eg. struct nbd_handle *, int, char * > * => [ true, false, true ] > @@ -403,6 +413,13 @@ let > pr "#endif\n"; > pr "#endif /* ! defined LIBNBD_ATTRIBUTE_NONNULL */\n"; > pr "\n"; > + pr "#if defined(__GNUC__) && LIBNBD_GCC_VERSION >= 120000 /* gcc >= 12.0 > */\n"; > + pr "#define LIBNBD_ATTRIBUTE_ALLOC_DEALLOC(fn) \\\n"; > + pr " __attribute__((__malloc__, __malloc__(fn, 1)))\n"; > + pr "#else\n"; > + pr "#define LIBNBD_ATTRIBUTE_ALLOC_DEALLOC(fn)\n"; > + pr "#endif\n"; > + pr "\n"; > pr "struct nbd_handle;\n"; > pr "\n"; > List.iter ( > @@ -433,12 +450,13 @@ let > pr "#define %-40s %d\n" n i > ) constants; > pr "\n"; > - pr "extern struct nbd_handle *nbd_create (void);\n"; > - pr "#define LIBNBD_HAVE_NBD_CREATE 1\n"; > - pr "\n"; > pr "extern void nbd_close (struct nbd_handle *h); /* h can be NULL */\n"; > pr "#define LIBNBD_HAVE_NBD_CLOSE 1\n"; > pr "\n"; > + pr "extern struct nbd_handle *nbd_create (void)\n"; > + pr " LIBNBD_ATTRIBUTE_ALLOC_DEALLOC(nbd_close);\n"; > + pr "#define LIBNBD_HAVE_NBD_CREATE 1\n"; > + pr "\n"; > pr "extern const char *nbd_get_error (void);\n"; > pr "#define LIBNBD_HAVE_NBD_GET_ERROR 1\n"; > pr "\n";
ACK - worth a go, if it causes too many problems we can always back it out later! Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org _______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
