[Libguestfs] [nbdkit PATCH v2 5/7] server: Allow longer NBD_OPT
Richard W.M. Jones
rjones at redhat.com
Sat Sep 28 21:38:32 UTC 2019
On Fri, Sep 27, 2019 at 11:48:47PM -0500, Eric Blake wrote:
> Fixes the fact that clients could not request the maximum string
> length except with NBD_OPT_EXPORT_LEN. Updates the testsuite to
> match.
>
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
> server/protocol-handshake-newstyle.c | 12 +++++++-----
> tests/test-long-name.sh | 10 ++++------
> 2 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/server/protocol-handshake-newstyle.c b/server/protocol-handshake-newstyle.c
> index 34958360..3b5d144e 100644
> --- a/server/protocol-handshake-newstyle.c
> +++ b/server/protocol-handshake-newstyle.c
> @@ -48,7 +48,7 @@
> #define MAX_NR_OPTIONS 32
>
> /* Maximum length of any option data (bytes). */
> -#define MAX_OPTION_LENGTH 4096
> +#define MAX_OPTION_LENGTH (NBD_MAX_STRING * 4)
>
> /* Receive newstyle options. */
> static int
> @@ -255,7 +255,7 @@ negotiate_handshake_newstyle_options (struct connection *conn)
> uint64_t version;
> uint32_t option;
> uint32_t optlen;
> - char data[MAX_OPTION_LENGTH+1];
> + CLEANUP_FREE char *data = NULL;
Even though you have the CLEANUP here ...
> struct nbd_export_name_option_reply handshake_finish;
> const char *optname;
> uint64_t exportsize;
> @@ -281,6 +281,11 @@ negotiate_handshake_newstyle_options (struct connection *conn)
> nbdkit_error ("client option data too long (%" PRIu32 ")", optlen);
> return -1;
> }
> + data = malloc (optlen + 1); /* Allowing a trailing NUL helps some uses */
> + if (data == NULL) {
> + nbdkit_error ("malloc: %m");
> + return -1;
> + }
... when I run this patch series under valgrind I get mainly errors
originating at this malloc:
==1251605== 58 bytes in 4 blocks are definitely lost in loss record 4 of 10
==1251605== at 0x896180B: malloc (vg_replace_malloc.c:309)
==1251605== by 0x11909F: protocol_handshake_newstyle (protocol-handshake-news
tyle.c:288)
==1251605== by 0x118804: protocol_handshake (protocol-handshake.c:55)
==1251605== by 0x112080: handle_single_connection (connections.c:165)
==1251605== by 0x11B84D: start_thread (sockets.c:276)
==1251605== by 0x8BB74E1: start_thread (pthread_create.c:479)
==1251605== by 0x8CD3642: clone (clone.S:95)
I didn't look at it closely but there does appear to be a memory leak
in this patch.
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
More information about the Libguestfs
mailing list