[ruby] Fix use-after-free for post-fork channel recreation (#35488)

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 c1813cee01
PiperOrigin-RevId: 599304551
pull/35586/head
Alan Wu 1 year ago committed by Copybara-Service
parent b34d98fbd4
commit b85ebb9bba
  1. 64
      src/ruby/end2end/fork_test_repro_35489.rb
  2. 4
      src/ruby/ext/grpc/rb_channel_args.c
  3. 1
      tools/run_tests/run_tests.py

@ -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

@ -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);

@ -1026,6 +1026,7 @@ class RubyLanguage(object):
"src/ruby/end2end/bad_usage_fork_test.rb",
"src/ruby/end2end/prefork_without_using_grpc_test.rb",
"src/ruby/end2end/prefork_postfork_loop_test.rb",
"src/ruby/end2end/fork_test_repro_35489.rb",
]:
# Skip fork tests in general until https://github.com/grpc/grpc/issues/34442
# is fixed. Otherwise we see too many flakes.

Loading…
Cancel
Save