Bug #128840 for grpc-xs: t/01-call_credentials.t crashes with "double free or corruption (fasttop)" on ppc64le, s390x, and armv7hl

Fri Mar 15 04:57:55 2019 ppisar [...] redhat.com - Ticket created

Subject:

t/01-call_credentials.t crashes with "double free or corruption (fasttop)" on ppc64le, s390x, and armv7hl

t/01-call_credentials.t test crashes always on ppc64le, s390x, and armv7hl platforms with "double free or corruption (fasttop)". It passes on x86_64, i686 and aarch64. I don't have a back trace yet, but a compiler warning seems to highlight a possible reason: mv Grpc.xsc Grpc.c [...] gcc -c -I. -D_REENTRANT -D_GNU_SOURCE -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mcpu=power8 -mtune=power8 -fasynchronous-unwind-tables -fstack-clash-protection -fwrapv -fno-strict-aliasing -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mcpu=power8 -mtune=power8 -fasynchronous-unwind-tables -fstack-clash-protection -DVERSION=\"0.20\" -DXS_VERSION=\"0.20\" -fPIC "-I/usr/lib64/perl5/CORE" -DGRPC_VERSION_1_1 -DGRPC_VERSION_1_2 -DGRPC_VERSION_1_4 -DGRPC_RECV_STATUS_ON_CLIENT_HAS_ERROR_STRING -DGRPC_SSL_CREDENTIALS_HAS_4_ARGS Grpc.c [...] BUILDSTDERR: ./ext/call_credentials.xs: In function 'XS_Grpc__XS__CallCredentials_createFromPlugin': BUILDSTDERR: ./ext/call_credentials.xs:19:25: warning: assignment to 'int (*)(void *, grpc_auth_metadata_context, void (*)(void *, const grpc_metadata *, size_t, grpc_status_code, const char *), void *, grpc_metadata *, size_t *, grpc_status_code *, const char **)' {aka 'int (*)(void *, struct <anonymous>, void (*)(void *, const struct grpc_metadata *, long unsigned int, enum <anonymous>, const char *), void *, struct grpc_metadata *, long unsigned int *, enum <anonymous> *, const char **)'} from incompatible pointer type 'void (*)(void *, grpc_auth_metadata_context, void (*)(void *, const grpc_metadata *, size_t, grpc_status_code, const char *), void *)' {aka 'void (*)(void *, struct <anonymous>, void (*)(void *, const struct grpc_metadata *, long unsigned int, enum <anonymous>, const char *), void *)'} [-Wincompatible-pointer-types] BUILDSTDERR: 19 | plugin.get_metadata = plugin_get_metadata; BUILDSTDERR: | ^ Although the warning presents on all platforms. I have perl 5.28.1, gcc 9.0.1, glibc 2.29.9000, grpc 1.18.0.

Fri Mar 15 07:56:36 2019 ppisar [...] redhat.com - Correspondence added

Dne Pá 15.bře.2019 04:57:55, ppisar napsal(a): Show quoted text

> t/01-call_credentials.t test crashes always on ppc64le, s390x, and > armv7hl platforms with "double free or corruption (fasttop)".

The test prints: 1..28 ok 1 - use Grpc::XS::CallCredentials; ok 2 - use Grpc::XS::Server; ok 3 - use Grpc::XS::ChannelCredentials; ok 4 - use Grpc::XS::ServerCredentials; ok 5 - use Grpc::Constants; ok 6 - call back func failed service_url ok 7 - call back func failed method_name double free or corruption (fasttop) Aborted (core dumped) A back trace is: 0x00007ffff7a12588 in __libc_signal_restore_set (set=0x7fffffffd498) at ../sysdeps/unix/sysv/linux/internal-signals.h:84 84 return INTERNAL_SYSCALL (rt_sigprocmask, err, 4, SIG_SETMASK, set, NULL, (gdb) bt #0 0x00007ffff7a12588 in __libc_signal_restore_set (set=0x7fffffffd498) at ../sysdeps/unix/sysv/linux/internal-signals.h:84 #1 __GI_raise (sig=<optimized out>) at ../sysdeps/unix/sysv/linux/raise.c:48 #2 0x00007ffff79f4710 in __GI_abort () at abort.c:79 #3 0x00007ffff7a5dbbc in __libc_message (action=<optimized out>, fmt=<optimized out>) at ../sysdeps/posix/libc_fatal.c:181 #4 0x00007ffff7a675c8 in malloc_printerr (str=<optimized out>, str@entry=0x7ffff7b78640 "double free or corruption (fasttop)") at malloc.c:5352 #5 0x00007ffff7a69860 in _int_free (av=0x7ffff7bd0dc0 <main_arena>, p=0x100acfa10, have_lock=<optimized out>) at malloc.c:4274 #6 0x00007ffff71fe268 in gpr_free () from /lib64/libgrpc.so.7 #7 0x00007ffff717f55c in grpc_plugin_credentials::get_request_metadata(grpc_polling_entity*, grpc_auth_metadata_context, grpc_credentials_mdelem_array*, grpc_closure*, grpc_error**) () from /lib64/libgrpc.so.7 #8 0x00007ffff718c534 in ?? () from /lib64/libgrpc.so.7 #9 0x00007ffff718c9b4 in ?? () from /lib64/libgrpc.so.7 #10 0x00007ffff70e891c in grpc_call_next_op(grpc_call_element*, grpc_transport_stream_op_batch*) () from /lib64/libgrpc.so.7 #11 0x00007ffff71fd268 in ?? () from /lib64/libgrpc.so.7 #12 0x00007ffff71c69b0 in grpc_subchannel_call_process_op(grpc_subchannel_call*, grpc_transport_stream_op_batch*) () from /lib64/libgrpc.so.7 #13 0x00007ffff71ac150 in ?? () from /lib64/libgrpc.so.7 #14 0x00007ffff7106dc0 in grpc_core::ExecCtx::Flush() () from /lib64/libgrpc.so.7 #15 0x00007ffff70feb40 in ?? () from /lib64/libgrpc.so.7 #16 0x00007ffff710538c in ?? () from /lib64/libgrpc.so.7 #17 0x00007ffff710b280 in grpc_pollset_work(grpc_pollset*, grpc_pollset_worker**, long) () from /lib64/libgrpc.so.7 #18 0x00007ffff7138a70 in ?? () from /lib64/libgrpc.so.7 #19 0x00007ffff7138fa8 in grpc_completion_queue_pluck () from /lib64/libgrpc.so.7 #20 0x00007ffff72aba90 in XS_Grpc__XS__Call_startBatch (my_perl=0x100030260, cv=<optimized out>) at ./ext/call.xs:266 #21 0x00007ffff7d0c274 in Perl_pp_entersub () from /lib64/libperl.so.5.28 #22 0x00007ffff7d0020c in Perl_runops_standard () from /lib64/libperl.so.5.28 #23 0x00007ffff7c5a7bc in perl_run () from /lib64/libperl.so.5.28 #24 0x0000000100000ee0 in main (argc=<optimized out>, argv=<optimized out>, env=<optimized out>) at perlmain.c:122 So it crashes on gpr_free() called from the end of grpc_plugin_credentials::get_request_metadata() function. The function modifies gpr_free()-d error_details pointer by this code: if (!plugin_.get_metadata( plugin_.state, context, plugin_md_request_metadata_ready, request, creds_md, &num_creds_md, &status, &error_details)) { [...] } This I believe this a mismatch on the plugin.get_metadata prototype and plugin_get_metadata() function provided by grpc-xs in util.c as warned by the compiler. It manifests only on platforms where a calling convention passes clobbers the unspecified arguments like the final error_details.

Fri Mar 15 10:00:00 2019 ppisar [...] redhat.com - Correspondence added

Dne Pá 15.bře.2019 07:56:36, ppisar napsal(a): Show quoted text

> This I believe this a mismatch on the plugin.get_metadata prototype > and plugin_get_metadata() function provided by grpc-xs in util.c as > warned by the compiler.

An attached patch fixes it.

Subject:

grpc-xs-0.20-Adapt-to-plugin-credentials-API-change-in-gRPC.patch

From 5a4288439361bc617b453435b048d9ea8d2b51a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20P=C3=ADsa=C5=99?= <ppisar@redhat.com> Date: Fri, 15 Mar 2019 13:29:49 +0100 Subject: [PATCH] Adapt to plugin credentials API change in gRPC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit gRPC changed grpc_metadata_credentials_plugin.get_metadata callback protype in 1.7.0 version with: commit 2caf021772ee241da3366e7dfd32aa4ee1134092 Author: Mark D. Roth <roth@google.com> Date: Fri Sep 1 15:04:13 2017 -0700 Change plugin credentials API to support both sync and async modes. That caused a mismatch between gRPC and grpc-xs and that triggered crashes in t/01-call_credentials.t test on ppc64le, s390x, and armv7hl platforms. This patch fixes it. CPAN RT#128840 Signed-off-by: Petr PÃsaÅ <ppisar@redhat.com> --- Makefile.PL | 8 ++++++++ util.c | 12 ++++++++++++ util.h | 9 +++++++++ 3 files changed, 29 insertions(+) diff --git a/Makefile.PL b/Makefile.PL index 76a50d1..fb0ba82 100644 --- a/Makefile.PL +++ b/Makefile.PL @@ -62,6 +62,14 @@ exit(0); EOT ) and ($EXTRA_DEFINES .= " -DGRPC_VERSION_1_4"); +check_lib( + %CHECKLIB_ARGS, + header => 'grpc/grpc_security.h', + function => <<'EOT', +return !GRPC_METADATA_CREDENTIALS_PLUGIN_SYNC_MAX; +EOT +) and ($EXTRA_DEFINES .= " -DGRPC_VERSION_1_7"); + check_lib( %CHECKLIB_ARGS, function => <<'EOT', diff --git a/util.c b/util.c index 3127a69..498632c 100644 --- a/util.c +++ b/util.c @@ -235,9 +235,18 @@ bool create_metadata_array(HV *hash, grpc_metadata_array *metadata) { } /* Callback function for plugin creds API */ +#if defined(GRPC_VERSION_1_7) +int plugin_get_metadata(void *ptr, grpc_auth_metadata_context context, + grpc_credentials_plugin_metadata_cb cb, + void *user_data, + grpc_metadata creds_md[GRPC_METADATA_CREDENTIALS_PLUGIN_SYNC_MAX], + size_t *num_creds_md, grpc_status_code *status, + const char **error_details) { +#else void plugin_get_metadata(void *ptr, grpc_auth_metadata_context context, grpc_credentials_plugin_metadata_cb cb, void *user_data) { +#endif SV* callback = (SV*)ptr; dSP; @@ -274,6 +283,9 @@ void plugin_get_metadata(void *ptr, grpc_auth_metadata_context context, /* Pass control back to core */ cb(user_data, metadata.metadata, metadata.count, code, NULL); +#if defined(GRPC_VERSION_1_7) + return 0; +#endif } /* Cleanup function for plugin creds API */ diff --git a/util.h b/util.h index 8955435..9037536 100644 --- a/util.h +++ b/util.h @@ -30,9 +30,18 @@ void perl_grpc_read_args_array(HV *hash, grpc_channel_args *args); HV* grpc_parse_metadata_array(grpc_metadata_array *metadata_array); bool create_metadata_array(HV *hash, grpc_metadata_array *metadata); +#if defined(GRPC_VERSION_1_7) +int plugin_get_metadata(void *ptr, grpc_auth_metadata_context context, + grpc_credentials_plugin_metadata_cb cb, + void *user_data, + grpc_metadata creds_md[GRPC_METADATA_CREDENTIALS_PLUGIN_SYNC_MAX], + size_t *num_creds_md, grpc_status_code *status, + const char **error_details); +#else void plugin_get_metadata(void *ptr, grpc_auth_metadata_context context, grpc_credentials_plugin_metadata_cb cb, void *user_data); +#endif void plugin_destroy_state(void *ptr); -- 2.20.1

Fri Mar 15 18:10:26 2019 joyrex2001 [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #128840] t/01-call_credentials.t crashes with "double free or corruption (fasttop)" on ppc64le, s390x, and armv7hl
Date:	Fri, 15 Mar 2019 23:04:07 +0100
To:	Petr Pisar via RT <bug-grpc-xs [...] rt.cpan.org>
From:	Vincent van Dam <joyrex2001 [...] gmail.com>

Thanks for the fix! The fix has been added to 0.30. Gr., Vincent Show quoted text

> On 15 Mar 2019, at 15:00, Petr Pisar via RT <bug-grpc-xs@rt.cpan.org> wrote: > > Queue: grpc-xs > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=128840 > > > Dne Pá 15.bře.2019 07:56:36, ppisar napsal(a):

>> This I believe this a mismatch on the plugin.get_metadata prototype >> and plugin_get_metadata() function provided by grpc-xs in util.c as >> warned by the compiler.

> > An attached patch fixes it. > > > From 5a4288439361bc617b453435b048d9ea8d2b51a1 Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Petr=20P=C3=ADsa=C5=99?= <ppisar@redhat.com> > Date: Fri, 15 Mar 2019 13:29:49 +0100 > Subject: [PATCH] Adapt to plugin credentials API change in gRPC > MIME-Version: 1.0 > Content-Type: text/plain; charset=UTF-8 > Content-Transfer-Encoding: 8bit > > gRPC changed grpc_metadata_credentials_plugin.get_metadata callback > protype in 1.7.0 version with: > > commit 2caf021772ee241da3366e7dfd32aa4ee1134092 > Author: Mark D. Roth <roth@google.com> > Date: Fri Sep 1 15:04:13 2017 -0700 > > Change plugin credentials API to support both sync and async modes. > > That caused a mismatch between gRPC and grpc-xs and that triggered > crashes in t/01-call_credentials.t test on ppc64le, s390x, and armv7hl > platforms. > > This patch fixes it. > > CPAN RT#128840 > > Signed-off-by: Petr Písař <ppisar@redhat.com> > --- > Makefile.PL | 8 ++++++++ > util.c | 12 ++++++++++++ > util.h | 9 +++++++++ > 3 files changed, 29 insertions(+) > > diff --git a/Makefile.PL b/Makefile.PL > index 76a50d1..fb0ba82 100644 > --- a/Makefile.PL > +++ b/Makefile.PL > @@ -62,6 +62,14 @@ exit(0); > EOT > ) and ($EXTRA_DEFINES .= " -DGRPC_VERSION_1_4"); > > +check_lib( > + %CHECKLIB_ARGS, > + header => 'grpc/grpc_security.h', > + function => <<'EOT', > +return !GRPC_METADATA_CREDENTIALS_PLUGIN_SYNC_MAX; > +EOT > +) and ($EXTRA_DEFINES .= " -DGRPC_VERSION_1_7"); > + > check_lib( > %CHECKLIB_ARGS, > function => <<'EOT', > diff --git a/util.c b/util.c > index 3127a69..498632c 100644 > --- a/util.c > +++ b/util.c > @@ -235,9 +235,18 @@ bool create_metadata_array(HV *hash, grpc_metadata_array *metadata) { > } > > /* Callback function for plugin creds API */ > +#if defined(GRPC_VERSION_1_7) > +int plugin_get_metadata(void *ptr, grpc_auth_metadata_context context, > + grpc_credentials_plugin_metadata_cb cb, > + void *user_data, > + grpc_metadata creds_md[GRPC_METADATA_CREDENTIALS_PLUGIN_SYNC_MAX], > + size_t *num_creds_md, grpc_status_code *status, > + const char **error_details) { > +#else > void plugin_get_metadata(void *ptr, grpc_auth_metadata_context context, > grpc_credentials_plugin_metadata_cb cb, > void *user_data) { > +#endif > SV* callback = (SV*)ptr; > > dSP; > @@ -274,6 +283,9 @@ void plugin_get_metadata(void *ptr, grpc_auth_metadata_context context, > > /* Pass control back to core */ > cb(user_data, metadata.metadata, metadata.count, code, NULL); > +#if defined(GRPC_VERSION_1_7) > + return 0; > +#endif > } > > /* Cleanup function for plugin creds API */ > diff --git a/util.h b/util.h > index 8955435..9037536 100644 > --- a/util.h > +++ b/util.h > @@ -30,9 +30,18 @@ void perl_grpc_read_args_array(HV *hash, grpc_channel_args *args); > HV* grpc_parse_metadata_array(grpc_metadata_array *metadata_array); > bool create_metadata_array(HV *hash, grpc_metadata_array *metadata); > > +#if defined(GRPC_VERSION_1_7) > +int plugin_get_metadata(void *ptr, grpc_auth_metadata_context context, > + grpc_credentials_plugin_metadata_cb cb, > + void *user_data, > + grpc_metadata creds_md[GRPC_METADATA_CREDENTIALS_PLUGIN_SYNC_MAX], > + size_t *num_creds_md, grpc_status_code *status, > + const char **error_details); > +#else > void plugin_get_metadata(void *ptr, grpc_auth_metadata_context context, > grpc_credentials_plugin_metadata_cb cb, > void *user_data); > +#endif > > void plugin_destroy_state(void *ptr); > > -- > 2.20.1 >

Fri Mar 15 18:10:26 2019 The RT System itself - Status changed from 'new' to 'open'