Merge pull request grpc#14127 from ZhouyihaiDing/channel_credentials_…

…leak php: fix channel_credentials hashstr leak
sheenaqotj · Jan 23, 2018 · 2318b87 · 2318b87
2 parents d715e7d + 4b9f8d8
commit 2318b87
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 2 deletions.
diff --git a/src/php/ext/grpc/channel.c b/src/php/ext/grpc/channel.c
@@ -69,6 +69,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);
     }
@@ -277,9 +281,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);
 
@@ -304,6 +313,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;
     }
   }
@@ -418,6 +432,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
@@ -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);
 }