Skip to content

Commit

Permalink
Fix file descriptor passing problems.
Browse files Browse the repository at this point in the history
When the ApplicationSpawner server spawns a worker process, do not let it
the worker process communicate directly with the process that issued the
spawn command to the ApplicationSpawner server. This is because upon receiving
the owner pipe, the issuer process will send a message to the
ApplicationSpawner server, as part of the new FD negotiation protocol. If the
ApplicationSpawner server has gone back into its message multiplexing main
loop then it'll crash.

Instead, the ApplicationSpawner server will now act as a proxy between the
issuer and the worker process.
  • Loading branch information
FooBarWidget committed Feb 11, 2010
1 parent 157be43 commit c056b19
Show file tree
Hide file tree
Showing 8 changed files with 83 additions and 10 deletions.
23 changes: 21 additions & 2 deletions ext/common/MessageChannel.h
Original file line number Diff line number Diff line change
Expand Up @@ -319,12 +319,23 @@ class MessageChannel {
* descriptor is a Unix socket.
*
* @param fileDescriptor The file descriptor to pass.
* @param negotiate See Ruby's MessageChannel#send_io method's comments.
* @throws SystemException Something went wrong during file descriptor passing.
* @throws boost::thread_interrupted
* @pre <tt>fileDescriptor >= 0</tt>
* @see readFileDescriptor()
*/
void writeFileDescriptor(int fileDescriptor) {
void writeFileDescriptor(int fileDescriptor, bool negotiate = true) {
if (negotiate) {
vector<string> args;

if (!read(args)) {
throw IOException("Unexpected end of stream encountered");
} else if (args.size() != 1 || args[0] != "pass IO") {
throw IOException("FD passing negotiation message expected.");
}
}

struct msghdr msg;
struct iovec vec;
char dummy[1];
Expand Down Expand Up @@ -492,6 +503,7 @@ class MessageChannel {
* Receive a file descriptor, which had been passed over the underlying
* file descriptor.
*
* @param negotiate See Ruby's MessageChannel#send_io method's comments.
* @return The passed file descriptor.
* @throws SystemException If something went wrong during the
* receiving of a file descriptor. Perhaps the underlying
Expand All @@ -500,7 +512,11 @@ class MessageChannel {
* file descriptor.
* @throws boost::thread_interrupted
*/
int readFileDescriptor() {
int readFileDescriptor(bool negotiate = true) {
if (negotiate) {
write("pass IO", NULL);
}

struct msghdr msg;
struct iovec vec;
char dummy[1];
Expand Down Expand Up @@ -538,6 +554,9 @@ class MessageChannel {
}

control_header = CMSG_FIRSTHDR(&msg);
if (control_header == NULL) {
throw IOException("No valid file descriptor received.");
}
if (control_header->cmsg_len != EXPECTED_CMSG_LEN
|| control_header->cmsg_level != SOL_SOCKET
|| control_header->cmsg_type != SCM_RIGHTS) {
Expand Down
6 changes: 5 additions & 1 deletion ext/phusion_passenger/native_support.c
Original file line number Diff line number Diff line change
Expand Up @@ -158,10 +158,14 @@ recv_fd(VALUE self, VALUE socket_fd) {
}

control_header = CMSG_FIRSTHDR(&msg);
if (control_header == NULL) {
rb_raise(rb_eIOError, "No valid file descriptor received.");
return Qnil;
}
if (control_header->cmsg_len != EXPECTED_CMSG_LEN
|| control_header->cmsg_level != SOL_SOCKET
|| control_header->cmsg_type != SCM_RIGHTS) {
rb_sys_fail("No valid file descriptor received.");
rb_raise(rb_eIOError, "No valid file descriptor received.");
return Qnil;
}
#if defined(__APPLE__) || defined(__SOLARIS__) || defined(__arm__)
Expand Down
30 changes: 29 additions & 1 deletion lib/phusion_passenger/message_channel.rb
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ def write_scalar(data)
# Might raise SystemCallError, IOError or SocketError when something
# goes wrong.
def recv_io
write("pass IO")
return @io.recv_io
end

Expand All @@ -204,7 +205,34 @@ def recv_io
# Might raise SystemCallError, IOError or SocketError when something
# goes wrong.
def send_io(io)
@io.send_io(io)
# We read a message before actually calling #send_io
# in order to prevent the other side from accidentally
# read()ing past the normal data and reading our file
# descriptor too.
#
# For example suppose that side A looks like this:
#
# read(fd, buf, 1024)
# read_io(fd)
#
# and side B:
#
# write(fd, buf, 100)
# send_io(fd_to_pass)
#
# If B completes both write() and send_io(), then A's read() call
# reads past the 100 bytes that B sent. On some platforms, like
# Linux, this will cause read_io() to fail. And it just so happens
# that Ruby's IO#read method slurps more than just the given amount
# of bytes.
result = read
if !result
raise EOFError, "End of stream"
elsif result != ["pass IO"]
raise IOError, "IO passing header expected"
else
@io.send_io(io)
end
end

# Return the file descriptor of the underlying IO object.
Expand Down
16 changes: 15 additions & 1 deletion lib/phusion_passenger/railz/application_spawner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -346,16 +346,30 @@ def rails_will_preload_app_code?
end

def handle_spawn_application
a, b = UNIXSocket.pair
safe_fork('application', true) do
begin
start_request_handler(client, true)
a.close
client.close
start_request_handler(MessageChannel.new(b), true)
rescue SignalException => e
if e.message != AbstractRequestHandler::HARD_TERMINATION_SIGNAL &&
e.message != AbstractRequestHandler::SOFT_TERMINATION_SIGNAL
raise
end
end
end

b.close
worker_channel = MessageChannel.new(a)
info = worker_channel.read
owner_pipe = worker_channel.recv_io
client.write(*info)
client.send_io(owner_pipe)
ensure
a.close if a
b.close if b && !b.closed?
owner_pipe.close if owner_pipe
end

# Initialize the request handler and enter its main loop.
Expand Down
10 changes: 9 additions & 1 deletion test/MessageChannelTest.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#include "tut.h"
#include "MessageChannel.h"

#include <boost/bind.hpp>
#include <boost/thread.hpp>
#include <cstring>
#include <cstdio>

Expand Down Expand Up @@ -144,8 +146,14 @@ namespace tut {
MessageChannel channel2(s[1]);

pipe(my_pipe);
channel1.writeFileDescriptor(my_pipe[1]);
boost::thread thr(boost::bind(
&MessageChannel::writeFileDescriptor,
&channel1,
my_pipe[1],
true
));
fd = channel2.readFileDescriptor();
thr.join();

char buf[5];
write(fd, "hello", 5);
Expand Down
2 changes: 1 addition & 1 deletion test/stub/message_channel.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#!/usr/bin/env ruby
$LOAD_PATH << "#{File.dirname(__FILE__)}/../../lib"
$LOAD_PATH.unshift("#{File.dirname(__FILE__)}/../../lib")
require 'phusion_passenger/message_channel'

include PhusionPassenger
Expand Down
2 changes: 1 addition & 1 deletion test/stub/message_channel_2.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#!/usr/bin/env ruby
$LOAD_PATH << "#{File.dirname(__FILE__)}/../../lib"
$LOAD_PATH.unshift("#{File.dirname(__FILE__)}/../../lib")
require 'phusion_passenger/message_channel'

include PhusionPassenger
Expand Down
4 changes: 2 additions & 2 deletions test/stub/message_channel_3.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#!/usr/bin/env ruby
$LOAD_PATH << "#{File.dirname(__FILE__)}/../../lib"
$LOAD_PATH << "#{File.dirname(__FILE__)}/../../ext"
$LOAD_PATH.unshift("#{File.dirname(__FILE__)}/../../lib")
$LOAD_PATH.unshift("#{File.dirname(__FILE__)}/../../ext")
require 'phusion_passenger/message_channel'
require 'phusion_passenger/utils'

Expand Down

0 comments on commit c056b19

Please sign in to comment.