From 4b9f8d802468e2eda825853206ffe6f1b5e72956 Mon Sep 17 00:00:00 2001 From: Zhouyihai Ding Date: Mon, 22 Jan 2018 12:58:21 -0800 Subject: [PATCH] php: channel_credentials hashstr leak --- src/php/ext/grpc/channel.c | 20 +++++++++++++++++++- src/php/ext/grpc/channel_credentials.c | 8 +++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/php/ext/grpc/channel.c b/src/php/ext/grpc/channel.c index db59869c7f7..325f592c554 100644 --- a/src/php/ext/grpc/channel.c +++ b/src/php/ext/grpc/channel.c @@ -68,6 +68,10 @@ PHP_GRPC_FREE_WRAPPED_FUNC_START(wrapped_grpc_channel) grpc_channel_destroy(p->wrapper->wrapped); free(p->wrapper->target); free(p->wrapper->args_hashstr); + if (p->wrapper->creds_hashstr != NULL) { + free(p->wrapper->creds_hashstr); + p->wrapper->creds_hashstr = NULL; + } } gpr_mu_unlock(&global_persistent_list_mu); } @@ -276,9 +280,14 @@ PHP_METHOD(Channel, __construct) { channel->wrapper->key = key; channel->wrapper->target = strdup(target); channel->wrapper->args_hashstr = strdup(sha1str); + channel->wrapper->creds_hashstr = NULL; if (creds != NULL && creds->hashstr != NULL) { - channel->wrapper->creds_hashstr = creds->hashstr; + php_grpc_int creds_hashstr_len = strlen(creds->hashstr); + char *channel_creds_hashstr = malloc(creds_hashstr_len + 1); + strcpy(channel_creds_hashstr, creds->hashstr); + channel->wrapper->creds_hashstr = channel_creds_hashstr; } + gpr_mu_init(&channel->wrapper->mu); smart_str_free(&buf); @@ -303,6 +312,11 @@ PHP_METHOD(Channel, __construct) { channel, target, args, creds, key, key_len TSRMLS_CC); } else { efree(args.args); + if (channel->wrapper->creds_hashstr != NULL){ + free(channel->wrapper->creds_hashstr); + channel->wrapper->creds_hashstr = NULL; + } + free(channel->wrapper->creds_hashstr); channel->wrapper = le->channel; } } @@ -416,6 +430,10 @@ PHP_METHOD(Channel, close) { grpc_channel_destroy(channel->wrapper->wrapped); free(channel->wrapper->target); free(channel->wrapper->args_hashstr); + if (channel->wrapper->creds_hashstr != NULL) { + free(channel->wrapper->creds_hashstr); + channel->wrapper->creds_hashstr = NULL; + } channel->wrapper->wrapped = NULL; php_grpc_delete_persistent_list_entry(channel->wrapper->key, diff --git a/src/php/ext/grpc/channel_credentials.c b/src/php/ext/grpc/channel_credentials.c index d120d6e90fe..ed74b991bd9 100644 --- a/src/php/ext/grpc/channel_credentials.c +++ b/src/php/ext/grpc/channel_credentials.c @@ -59,6 +59,7 @@ static grpc_ssl_roots_override_result get_ssl_roots_override( PHP_GRPC_FREE_WRAPPED_FUNC_START(wrapped_grpc_channel_credentials) if (p->wrapped != NULL) { grpc_channel_credentials_release(p->wrapped); + p->wrapped = NULL; } PHP_GRPC_FREE_WRAPPED_FUNC_END() @@ -199,8 +200,13 @@ PHP_METHOD(ChannelCredentials, createComposite) { grpc_channel_credentials *creds = grpc_composite_channel_credentials_create(cred1->wrapped, cred2->wrapped, NULL); + // wrapped_grpc_channel_credentials object should keeps it's own + // allocation. Otherwise it conflicts free hashstr with call.c. + php_grpc_int cred1_len = strlen(cred1->hashstr); + char *cred1_hashstr = malloc(cred1_len+1); + strcpy(cred1_hashstr, cred1->hashstr); zval *creds_object = - grpc_php_wrap_channel_credentials(creds, cred1->hashstr, true + grpc_php_wrap_channel_credentials(creds, cred1_hashstr, true TSRMLS_CC); RETURN_DESTROY_ZVAL(creds_object); }