aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorapolcyn <apolcyn@google.com>2024-01-19 13:40:08 -0800
committerGitHub <noreply@github.com>2024-01-19 13:40:08 -0800
commit3ce4476905709e763911c2af64f11901d69bc084 (patch)
treead21e956e23669535e3c0f2feebbd749a65d295e /src
parent8e92b0a8b8ba9dffa78fa991e5e738dce57273ea (diff)
downloadgrpc-grpc-3ce4476905709e763911c2af64f11901d69bc084.tar.gz
[ruby] backport "Fix use-after-free for post-fork channel recreation (#35488)" (#35613)
The `grpc_channel_args` is retained on the Ruby object and used for recreating the channel after forking in grpc_rb_channel_maybe_recreate_channel_after_fork(). Previously, the key for each argument was taken from a Ruby string directly, which could be invalidated if the Ruby string is modified or moved by the GC. Duplicate the string for the key instead, so we own it. Reproducer in https://github.com/grpc/grpc/issues/35489 <!-- If you know who should review your pull request, please assign it to that person, otherwise the pull request would get assigned randomly. If your pull request is for a specific language, please add the appropriate lang label. --> Closes #35488 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/35488 from Shopify:key-uaf c1813cee0184a5003f01dd1086df4f322917281b PiperOrigin-RevId: 599304551 <!-- If you know who should review your pull request, please assign it to that person, otherwise the pull request would get assigned randomly. If your pull request is for a specific language, please add the appropriate lang label. --> Co-authored-by: Alan Wu <XrXr@users.noreply.github.com>
Diffstat (limited to 'src')
-rwxr-xr-xsrc/ruby/end2end/fork_test_repro_35489.rb64
-rw-r--r--src/ruby/ext/grpc/rb_channel_args.c4
2 files changed, 67 insertions, 1 deletions
diff --git a/src/ruby/end2end/fork_test_repro_35489.rb b/src/ruby/end2end/fork_test_repro_35489.rb
new file mode 100755
index 0000000000..662a32f9a7
--- /dev/null
+++ b/src/ruby/end2end/fork_test_repro_35489.rb
@@ -0,0 +1,64 @@
+#!/usr/bin/env ruby
+#
+# Copyright 2016 gRPC authors.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+ENV['GRPC_ENABLE_FORK_SUPPORT'] = "1"
+fail "forking only supported on linux" unless RUBY_PLATFORM =~ /linux/
+
+this_dir = File.expand_path(File.dirname(__FILE__))
+protos_lib_dir = File.join(this_dir, 'lib')
+grpc_lib_dir = File.join(File.dirname(this_dir), 'lib')
+$LOAD_PATH.unshift(grpc_lib_dir) unless $LOAD_PATH.include?(grpc_lib_dir)
+$LOAD_PATH.unshift(protos_lib_dir) unless $LOAD_PATH.include?(protos_lib_dir)
+$LOAD_PATH.unshift(this_dir) unless $LOAD_PATH.include?(this_dir)
+
+require 'grpc'
+require 'end2end_common'
+
+def do_rpc(stub)
+ stub.echo(Echo::EchoRequest.new(request: 'hello'), deadline: Time.now + 300)
+end
+
+def main
+ this_dir = File.expand_path(File.dirname(__FILE__))
+ echo_server_path = File.join(this_dir, 'echo_server.rb')
+ to_child_r, _to_child_w = IO.pipe
+ to_parent_r, to_parent_w = IO.pipe
+ # Note gRPC has not yet been initialized, otherwise we would need to call prefork
+ # before spawn and postfork_parent after.
+ # TODO(apolcyn): consider redirecting server's stderr to a file
+ Process.spawn(RbConfig.ruby, echo_server_path, in: to_child_r, out: to_parent_w, err: "server_log")
+ to_child_r.close
+ to_parent_w.close
+ child_port = to_parent_r.gets.strip
+ STDERR.puts "server running on port: #{child_port}"
+ key = "grpc." * 100_000 # large enough to malloc backing storage
+ value = "testvalue" * 100_000
+ stub = Echo::EchoServer::Stub.new("localhost:#{child_port}", :this_channel_is_insecure, channel_args: { key => value })
+ with_logging("parent: GRPC.prefork") { GRPC.prefork }
+ pid = fork do
+ with_logging("child: GRPC.postfork_child") { GRPC.postfork_child }
+ key.clear
+ value.clear
+ GC.compact
+ with_logging("child: post-fork RPC") { do_rpc(stub) }
+ STDERR.puts "child: done"
+ end
+ with_logging("parent: GRPC.postfork_parent") { GRPC.postfork_parent }
+ Process.wait pid
+ STDERR.puts "parent: done"
+end
+
+main
diff --git a/src/ruby/ext/grpc/rb_channel_args.c b/src/ruby/ext/grpc/rb_channel_args.c
index 8b7b176953..0dd43bcec9 100644
--- a/src/ruby/ext/grpc/rb_channel_args.c
+++ b/src/ruby/ext/grpc/rb_channel_args.c
@@ -71,7 +71,7 @@ static int grpc_rb_channel_create_in_process_add_args_hash_cb(VALUE key,
return ST_STOP;
}
- args->args[args->num_args - 1].key = (char*)the_key;
+ args->args[args->num_args - 1].key = gpr_strdup(the_key);
switch (TYPE(val)) {
case T_SYMBOL:
args->args[args->num_args - 1].type = GRPC_ARG_STRING;
@@ -163,6 +163,8 @@ void grpc_rb_channel_args_destroy(grpc_channel_args* args) {
GPR_ASSERT(args != NULL);
if (args->args == NULL) return;
for (int i = 0; i < args->num_args; i++) {
+ // the key was created with gpr_strdup
+ gpr_free(args->args[i].key);
if (args->args[i].type == GRPC_ARG_STRING) {
// we own string pointers, which were created with gpr_strdup
gpr_free(args->args[i].value.string);