diff options
author | apolcyn <apolcyn@google.com> | 2024-01-19 13:40:08 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-01-19 13:40:08 -0800 |
commit | 3ce4476905709e763911c2af64f11901d69bc084 (patch) | |
tree | ad21e956e23669535e3c0f2feebbd749a65d295e /src | |
parent | 8e92b0a8b8ba9dffa78fa991e5e738dce57273ea (diff) | |
download | grpc-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-x | src/ruby/end2end/fork_test_repro_35489.rb | 64 | ||||
-rw-r--r-- | src/ruby/ext/grpc/rb_channel_args.c | 4 |
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); |