search for: nbdkit_strdup_intern

Previously with GCC 10.2 this code produced: In function ‘nbdkit_strndup_intern’, inlined from ‘nbdkit_strdup_intern’ at public.c:839:10: public.c:827:10: error: ‘strndup’ specified bound 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Werror=stringop-overflow=] 827 | copy = strndup (str, n); | ^~~~~~~~~~~~~~~~ --- server/public.c | 16 +++++++++++++++- 1 file changed, 1...

I'm not sure if this is a GCC bug or a bug in our code, but the attached workaround fixes it for me. Rich.

On 9/3/20 4:41 AM, Richard W.M. Jones wrote: > Previously with GCC 10.2 this code produced: > > In function ‘nbdkit_strndup_intern’, > inlined from ‘nbdkit_strdup_intern’ at public.c:839:10: > public.c:827:10: error: ‘strndup’ specified bound 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Werror=stringop-overflow=] > 827 | copy = strndup (str, n); > | ^~~~~~~~~~~~~~~~ > --- > server/public.c | 16 +++++++...

A simple reproducer is: ---------------------------------------------------------------------- #include <stdio.h> #include <stdlib.h> #include <stdint.h> #include <string.h> const char * copyn (const char *str, size_t n) { return strndup (str, n); } const char * copy (const char *str) { return copyn (str, SIZE_MAX); }

Filed upstream as: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96916 Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW

On Thu, Sep 03, 2020 at 10:47:13AM +0100, Richard W.M. Jones wrote: > > A simple reproducer is: > > ---------------------------------------------------------------------- > #include <stdio.h> > #include <stdlib.h> > #include <stdint.h> > #include <string.h> > > const char * > copyn (const char *str, size_t n) > { > return strndup

...gt; function. However be careful when modifying strings because for some methods (eg. C<.config>) the plugin may save the string pointer that you pass along. So you may have to ensure that the string is not freed for the -lifetime of the server. +lifetime of the server; you may find C<nbdkit_strdup_intern> helpful +for avoiding a memory leak while still obeying lifecycle constraints. Note that if your filter registers a callback but in that callback it doesn't call the C<next> function then the corresponding method in the @@ -450,8 +451,10 @@ requires write access to the underlying d...

...ta); + CLEANUP_FREE char *desc = NULL; + + if (base) + asprintf (&desc, "embedded '%s%s' from within ext2 image: %s", + slash, fname, base); + else + asprintf (&desc, "embedded '%s%s' from within ext2 image", slash, fname); + return nbdkit_strdup_intern (desc); +} + /* Get the disk size. */ static int64_t ext2_get_size (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle) @@ -412,23 +431,24 @@ ext2_flush (struct nbdkit_next_ops *next_ops, void *nxdata, */ static struct nbdkit_filter filter = { - .name = "ext2&quo...

Applies on top of my pending series for the exportname filter, addressing one of the todo's in that cover letter. Eric Blake (2): filters: Add .export_description wrappers ext2: Supply .list_exports and .default_export filters/ext2/nbdkit-ext2-filter.pod | 3 +- tests/Makefile.am | 16 +++- filters/ext2/ext2.c | 125 +++++++++++++++++++---------

...up front isn't hard.  If API > proliferation is a problem, you can combine strdup and strndup by > accepting -1 as a length to mean stop at NUL termination; but I'm less > worried about API proliferation and more about ease-of-use. Time for some name bike-shedding. How about: nbdkit_strdup_intern (const char *) nbdkit_strndup_intern (const char *s, size_t) and if we need something that permits embedded NUL (right now I don't have such a use) we could add nbdkit_memdup_intern (const void *s, size_t) -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Vi...

...ULL; > + > + if (base) > + asprintf (&desc, "embedded '%s%s' from within ext2 image: %s", > + slash, fname, base); > + else > + asprintf (&desc, "embedded '%s%s' from within ext2 image", slash, fname); > + return nbdkit_strdup_intern (desc); > +} > + > /* Get the disk size. */ > static int64_t > ext2_get_size (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle) > @@ -412,23 +431,24 @@ ext2_flush (struct nbdkit_next_ops *next_ops, void *nxdata, > */ > > static struct nbdkit_filter filt...

This picks up python and ocaml. Some of our languages are lacking a number of bindings (for example, lua and perl lack .extents, so I didn't have anything to copy from), and I felt less comfortable with golang and rust. But for python and ocaml, I was able to test a working implementation. Eric Blake (2): python: Implement .list_exports and friends ocaml: Implement .list_exports and

Fairly straightforward. .list_exports uses the same idiom as .extents for returning an iterable of tuples, with additional support for a bare name rather than a name/desc tuple. .default_export and .export_description are rather easy clients of nbdkit_strdup_intern. Signed-off-by: Eric Blake <eblake at redhat.com> --- plugins/python/nbdkit-python-plugin.pod | 25 ++++ tests/Makefile.am | 3 + plugins/python/python.c | 185 ++++++++++++++++++++---- tests/test-python-export-list.sh | 69 +++++++++ tests/p...

...default_export_fn, Val_bool (readonly), + Val_bool (is_tls)); + if (Is_exception_result (rv)) { + nbdkit_error ("%s", caml_format_exception (Extract_exception (rv))); + caml_enter_blocking_section (); + CAMLreturnT (const char *, NULL); + } + + name = nbdkit_strdup_intern (String_val (rv)); + + caml_enter_blocking_section (); + CAMLreturnT (const char *, name); +} + static void * open_wrapper (int readonly) { @@ -358,6 +419,28 @@ close_wrapper (void *h) CAMLreturn0; } +static const char * +export_description_wrapper (void *h) +{ + const char *desc; + CAM...

...default_export_fn, Val_bool (readonly), + Val_bool (is_tls)); + if (Is_exception_result (rv)) { + nbdkit_error ("%s", caml_format_exception (Extract_exception (rv))); + caml_enter_blocking_section (); + CAMLreturnT (const char *, NULL); + } + + name = nbdkit_strdup_intern (String_val (rv)); + + caml_enter_blocking_section (); + CAMLreturnT (const char *, name); +} + static void * open_wrapper (int readonly) { @@ -358,6 +420,28 @@ close_wrapper (void *h) CAMLreturn0; } +static const char * +export_description_wrapper (void *h) +{ + const char *desc; + CAM...

When using ext2file=exportname to pick the file to server from the client's export name, we do not want to leak the underlying plugin's export list. While touching this, take advantage of string lifetimes via nbdkit_strdup_intern and similar for less cleanup bookkeeping. Note that we don't actually implement a full .list_exports; doing that with NBD_OPT_LIST is likely prohibitive (it's likely the disk contains LOTS of files), and would require that we can do next_ops->pread prior to the client reaching our .open...

...API > >proliferation is a problem, you can combine strdup and strndup by > >accepting -1 as a length to mean stop at NUL termination; but I'm > >less worried about API proliferation and more about ease-of-use. > > Time for some name bike-shedding. How about: > > nbdkit_strdup_intern (const char *) > nbdkit_strndup_intern (const char *s, size_t) > > and if we need something that permits embedded NUL (right now I > don't have such a use) we could add > nbdkit_memdup_intern (const void *s, size_t) Seems good to me. Do you think we'll be able to get the f...

This is a revision of my .default_export work, plus new work on .export_descriptions and a new exportname filter. I think it is now ready to check in. Things I'd still like in 1.22: - the file plugin should implement .list_exports (patch already posted, but it needs rebasing on this series) - the ext2 filter should override .list_exports when in exportname mode - the nbd plugin should be

...(struct sockaddr *addr, socklen_t *addrlen)); +NBDKIT_EXTERN_DECL (int, nbdkit_peer_pid, (void)); +NBDKIT_EXTERN_DECL (int, nbdkit_peer_uid, (void)); +NBDKIT_EXTERN_DECL (int, nbdkit_peer_gid, (void)); NBDKIT_EXTERN_DECL (void, nbdkit_shutdown, (void)); NBDKIT_EXTERN_DECL (const char *, nbdkit_strdup_intern, diff --git a/server/nbdkit.syms b/server/nbdkit.syms index 1eb18bb0..3d6b2235 100644 --- a/server/nbdkit.syms +++ b/server/nbdkit.syms @@ -67,7 +67,10 @@ nbdkit_parse_uint32_t; nbdkit_parse_uint64_t; nbdkit_parse_unsigned; + nbdkit_peer_gid; nbdkit_peer_name; + nbdkit_pee...

...tring (such as longer +than 4096 bytes), or if this callback is omitted, no description is +offered to the client. As this is not an error in the protocol, it is +not necessary to call C<nbdkit_error>. If the callback will not be +returning a compile-time constant string, you may find +C<nbdkit_strdup_intern> helpful for returning a value that avoids a +memory leak. + =head2 C<.can_write> int can_write (void *handle); diff --git a/docs/nbdkit-protocol.pod b/docs/nbdkit-protocol.pod index b923d367..2f46d735 100644 --- a/docs/nbdkit-protocol.pod +++ b/docs/nbdkit-protocol.pod @@ -200,6 +200,...