ipc: do not convert FDs to IO objects early

We now pass PerlIO scalar refs directly down a level to simplify
callers and avoid possible lifetime problems with FDs referencing
invalid IO objects.  These lifetime problems are not currently a
problem, supporting adding support for queueing SCM_RIGHTS
(FD-passing) requests via PublicInbox::WQBlocked will become a
problem in the near future.

diff --git a/lib/PublicInbox/CmdIPC4.pm b/lib/PublicInbox/CmdIPC4.pm
index af49f36d35710d3a385a1a346a90ef0312ab1b33..a5330e3d49ac90b99875e7c2dc02cb7e73ac3a60 100644 (file)
--- a/lib/PublicInbox/CmdIPC4.pm
+++ b/lib/PublicInbox/CmdIPC4.pm
@@ -23,11 +23,12 @@ require Socket::MsgHdr; # XS
 no warnings 'once';
 
 # any number of FDs per-sendmsg(2) + buffer
-*send_cmd4 = sub ($$$$;$) { # (sock, fds, buf, flags) = @_;
-       my ($sock, $fds, undef, $flags, $tries) = @_;
+*send_cmd4 = sub ($$$$;$) { # (sock, io, buf, flags) = @_;
+       my ($sock, $io, undef, $flags, $tries) = @_;
        $tries //= -1; # infinite
        my $mh = Socket::MsgHdr->new(buf => $_[2]);
-       $mh->cmsghdr(SOL_SOCKET, SCM_RIGHTS, pack('i' x scalar(@$fds), @$fds));
+       $mh->cmsghdr(SOL_SOCKET, SCM_RIGHTS,
+               pack('i' x scalar(@{$io //= []}), map { fileno $_ } @$io));
        my $s;
        do {
                $s = Socket::MsgHdr::sendmsg($sock, $mh, $flags);

diff --git a/lib/PublicInbox/IPC.pm b/lib/PublicInbox/IPC.pm
index 44e37bf16a7c993ad71fe46090df735717234e1c..b69d10b9f1f87ef50dfaf08efec3445589576757 100644 (file)
--- a/lib/PublicInbox/IPC.pm
+++ b/lib/PublicInbox/IPC.pm
@@ -395,29 +395,28 @@ sub wq_broadcast {
 }
 
 sub stream_in_full ($$$) {
-       my ($s1, $fds, $buf) = @_;
+       my ($s1, $io, $buf) = @_;
        socketpair(my $r, my $w, AF_UNIX, SOCK_STREAM, 0);
-       my $n = $send_cmd->($s1, [ fileno($r) ],
+       my $n = $send_cmd->($s1, [ $r ],
                        ipc_freeze(['do_sock_stream', length($buf)]),
                        MSG_EOR) // croak "sendmsg: $!";
        undef $r;
-       $n = $send_cmd->($w, $fds, $buf, 0) // croak "sendmsg: $!";
+       $n = $send_cmd->($w, $io, $buf, 0) // croak "sendmsg: $!";
        print $w substr($buf, $n) if $n < length($buf); # need > 2G on Linux
        close $w; # autodies
 }
 
 sub wq_io_do { # always async
-       my ($self, $sub, $ios, @args) = @_;
+       my ($self, $sub, $io, @args) = @_;
        my $s1 = $self->{-wq_s1} or Carp::confess('no -wq_s1');
-       my $fds = [ map { fileno($_) } @$ios ];
        my $buf = ipc_freeze([$sub, @args]);
        if (length($buf) > $MY_MAX_ARG_LEN) {
-               stream_in_full($s1, $fds, $buf);
+               stream_in_full($s1, $io, $buf);
        } else {
-               my $n = $send_cmd->($s1, $fds, $buf, MSG_EOR);
+               my $n = $send_cmd->($s1, $io, $buf, MSG_EOR);
                return if defined($n); # likely
                $!{ETOOMANYREFS} and croak "sendmsg: $! (check RLIMIT_NOFILE)";
-               $!{EMSGSIZE} ? stream_in_full($s1, $fds, $buf) :
+               $!{EMSGSIZE} ? stream_in_full($s1, $io, $buf) :
                        croak("sendmsg: $!");
        }
 }

diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index eb09424f8e1408763d1f35d2c2449de2b41647fb..3e965863c6e712032b3f0a06514f29116dde44d8 100644 (file)
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -1066,8 +1066,7 @@ sub send_exec_cmd { # tell script/lei to execute a command
        my ($self, $io, $cmd, $env) = @_;
        $PublicInbox::IPC::send_cmd->(
                        $self->{sock} // die('lei client gone'),
-                       [ map { fileno($_) } @$io ],
-                       exec_buf($cmd, $env), MSG_EOR) //
+                       $io, exec_buf($cmd, $env), MSG_EOR) //
                Carp::croak("sendmsg: $!");
 }
 

diff --git a/lib/PublicInbox/Spawn.pm b/lib/PublicInbox/Spawn.pm
index 9cb4865778d0e8101ebe569b5266d25ef19aca5e..e4480475d5b9311d40c040282cf92d15ecb1a96d 100644 (file)
--- a/lib/PublicInbox/Spawn.pm
+++ b/lib/PublicInbox/Spawn.pm
@@ -211,15 +211,15 @@ union my_cmsg {
        char pad[sizeof(struct cmsghdr) + 16 + SEND_FD_SPACE];
 };
 
-SV *send_cmd4_(PerlIO *s, SV *svfds, SV *data, int flags, long tries)
+SV *send_cmd4_(PerlIO *s, SV *sio, SV *data, int flags, long tries)
 {
        struct msghdr msg = { 0 };
        union my_cmsg cmsg = { 0 };
        STRLEN dlen = 0;
        struct iovec iov;
        ssize_t sent;
-       AV *fds = (AV *)SvRV(svfds);
-       I32 i, nfds = av_len(fds) + 1;
+       AV *io = (AV *)SvRV(sio);
+       I32 i, nfds = io ? (av_len(io) + 1) : 0;
        int *fdp;
 
        if (SvOK(data)) {
@@ -244,8 +244,8 @@ SV *send_cmd4_(PerlIO *s, SV *svfds, SV *data, int flags, long tries)
                cmsg.hdr.cmsg_len = CMSG_LEN(nfds * sizeof(int));
                fdp = (int *)CMSG_DATA(&cmsg.hdr);
                for (i = 0; i < nfds; i++) {
-                       SV **fd = av_fetch(fds, i, 0);
-                       *fdp++ = SvIV(*fd);
+                       SV **svio = av_fetch(io, i, 0);
+                       *fdp++ = PerlIO_fileno(IoIFP(sv_2io(*svio)));
                }
        }
        do {

diff --git a/lib/PublicInbox/Syscall.pm b/lib/PublicInbox/Syscall.pm
index fefb6b323d7d9993e5e5ec78fb845abe3b0c94f3..94658c7732043f0dd4a6a2f10471b33fa5138da3 100644 (file)
--- a/lib/PublicInbox/Syscall.pm
+++ b/lib/PublicInbox/Syscall.pm
@@ -526,18 +526,18 @@ if (defined($SYS_sendmsg) && defined($SYS_recvmsg)) {
 require PublicInbox::CmdIPC4;
 
 *send_cmd4 = sub ($$$$;$) {
-       my ($sock, $fds, undef, $flags, $tries) = @_;
+       my ($sock, $io, undef, $flags, $tries) = @_;
        my $iov = pack('P'.TMPL_size_t,
                        $_[2] // NUL, length($_[2] // NUL) || 1);
-       my $fd_space = scalar(@$fds) * SIZEOF_int;
+       my $fd_space = scalar(@{$io //= []}) * SIZEOF_int;
        my $msg_controllen = CMSG_SPACE($fd_space);
        my $cmsghdr = pack(TMPL_cmsg_len .
                        'LL' .  # cmsg_level, cmsg_type,
-                       CMSG_DATA_off.('i' x scalar(@$fds)). # CMSG_DATA
+                       CMSG_DATA_off.('i' x scalar(@$io)). # CMSG_DATA
                        '@'.($msg_controllen - 1).'x1', # pad to space, not len
                        CMSG_LEN($fd_space), # cmsg_len
                        SOL_SOCKET, SCM_RIGHTS, # cmsg_{level,type}
-                       @$fds); # CMSG_DATA
+                       map { fileno $_ } @$io); # CMSG_DATA
        my $mh = pack(TMPL_msghdr,
                        undef, 0, # msg_name, msg_namelen (unused)
                        $iov, 1, # msg_iov, msg_iovlen

diff --git a/lib/PublicInbox/XapClient.pm b/lib/PublicInbox/XapClient.pm
index 2790ae817925dacff5bd3f2de5336e9f98ad33ee..88638aed9e575e654c71b835191d0919a917bee7 100644 (file)
--- a/lib/PublicInbox/XapClient.pm
+++ b/lib/PublicInbox/XapClient.pm
@@ -15,11 +15,10 @@ use autodie qw(pipe socketpair);
 our $tries = -1; # set to zero by read-only daemon
 
 sub mkreq {
-       my ($self, $ios, @arg) = @_;
-       my @fds = map fileno($_), @$ios;
+       my ($self, $io, @arg) = @_;
        my $buf = join("\0", @arg, '');
        my $n = $PublicInbox::IPC::send_cmd->($self->{io},
-                               \@fds, $buf, MSG_EOR, $tries)
+                               $io, $buf, MSG_EOR, $tries)
                                // die "send_cmd: $!";
        $n == length($buf) or die "send_cmd: $n != ".length($buf);
 }

diff --git a/script/lei b/script/lei
index d504793c034ddda8c8e05a2067f5f3c864b839cf..9e8757619a33527d6ee787c5a8669dfa2efe5ce5 100755 (executable)
--- a/script/lei
+++ b/script/lei
@@ -109,7 +109,8 @@ open my $dh, '<', '.' or die "open(.) $!";
 my $buf = join("\0", scalar(@ARGV), @ARGV);
 while (my ($k, $v) = each %ENV) { $buf .= "\0$k=$v" }
 $buf .= "\0\0";
-$send_cmd->($sock, [0, 1, 2, fileno($dh)], $buf, MSG_EOR) or die "sendmsg: $!";
+$send_cmd->($sock, [\*STDIN, \*STDOUT, \*STDERR, $dh], $buf, MSG_EOR) or
+       die "sendmsg: $!";
 $SIG{TSTP} = sub { send($sock, 'STOP', MSG_EOR); kill 'STOP', $$ };
 $SIG{CONT} = sub { send($sock, 'CONT', MSG_EOR) };
 

diff --git a/t/cmd_ipc.t b/t/cmd_ipc.t
index a432b4f838a824299022e1e58f3aefee152062ed..df12864d17261dec172a4ec02c78bd15c81e2200 100644 (file)
--- a/t/cmd_ipc.t
+++ b/t/cmd_ipc.t
@@ -15,8 +15,8 @@ my $do_test = sub { SKIP: {
        my ($s1, $s2);
        my $src = 'some payload' x 40;
        socketpair($s1, $s2, AF_UNIX, $type, 0);
-       my $sfds = [ fileno($r), fileno($w), fileno($s1) ];
-       $send->($s1, $sfds, $src, $flag);
+       my $io = [ $r, $w, $s1 ];
+       $send->($s1, $io, $src, $flag);
        my (@fds) = $recv->($s2, my $buf, length($src) + 1);
        is($buf, $src, 'got buffer payload '.$desc);
        my ($r1, $w1, $s1a);
@@ -38,7 +38,7 @@ my $do_test = sub { SKIP: {
        if ($type == SOCK_SEQPACKET) {
                $r1 = $w1 = $s1a = undef;
                $src = (',' x 1023) . '-' .('.' x 1024);
-               $send->($s1, $sfds, $src, $flag);
+               $send->($s1, $io, $src, $flag);
                (@fds) = $recv->($s2, $buf, 1024);
                is($buf, (',' x 1023) . '-', 'silently truncated buf');
 
@@ -83,7 +83,7 @@ my $do_test = sub { SKIP: {
                $s1->blocking(0);
                my $nsent = 0;
                my $srclen = length($src);
-               while (defined(my $n = $send->($s1, $sfds, $src, $flag))) {
+               while (defined(my $n = $send->($s1, $io, $src, $flag))) {
                        $nsent += $n;
                        fail "sent $n bytes of $srclen" if $n > $srclen;
                }
@@ -93,12 +93,17 @@ my $do_test = sub { SKIP: {
                ok($nsent > 0, 'sent some bytes');
 
                socketpair($s1, $s2, AF_UNIX, $type, 0);
-               is($send->($s1, [], $src, $flag), length($src), 'sent w/o FDs');
+               is($send->($s1, [], $src, $flag), length($src), 'sent w/o IOs');
                $buf = 'nope';
                @fds = $recv->($s2, $buf, length($src));
                is(scalar(@fds), 0, 'no FDs received');
                is($buf, $src, 'recv w/o FDs');
        }
+       socketpair($s1, $s2, AF_UNIX, $type, 0);
+       is($send->($s1, undef, $src, $flag), length($src), 'sent w/ undef IO');
+       @fds = $recv->($s2, $buf = 'hi', length($src));
+       is scalar(@fds), 0, 'no FDs received';
+       is $buf, $src, 'recv w/o FDs sent buffer';
 } };
 
 my $send_ic = PublicInbox::Spawn->can('send_cmd4');

diff --git a/t/lei-daemon.t b/t/lei-daemon.t
index a9a95e0a7be9d3d1632cbbfa7a37ae795372f833..c07a35ede5ea123e388aaa376fa346cc01bc77be 100644 (file)
--- a/t/lei-daemon.t
+++ b/t/lei-daemon.t
@@ -35,12 +35,11 @@ test_lei({ daemon_only => 1 }, sub {
                my @before = sort(glob("$d/*"));
                my $addr = pack_sockaddr_un($sock);
                open my $null, '<', '/dev/null' or BAIL_OUT "/dev/null: $!";
-               my @fds = map { fileno($null) } (0..2);
                for (0..10) {
                        socket(my $c, AF_UNIX, SOCK_SEQPACKET, 0) or
                                                        BAIL_OUT "socket: $!";
                        connect($c, $addr) or BAIL_OUT "connect: $!";
-                       $send_cmd->($c, \@fds, 'hi', MSG_EOR);
+                       $send_cmd->($c, [ $null, $null, $null ], 'hi', MSG_EOR);
                }
                lei_ok('daemon-pid');
                chomp($pid = $lei_out);

diff --git a/t/xap_helper.t b/t/xap_helper.t
index 7455428dc808ce1ca0b69d177173b98ce38a2858..4e7b7f017071b544391e41c01406b52691ca6f81 100644 (file)
--- a/t/xap_helper.t
+++ b/t/xap_helper.t
@@ -90,8 +90,8 @@ my $doreq = sub {
        my $err = ref($arg[-1]) ? pop(@arg) : \*STDERR;
        pipe(my $x, my $y);
        my $buf = join("\0", @arg, '');
-       my @fds = (fileno($y), fileno($err));
-       my $n = $PublicInbox::IPC::send_cmd->($s, \@fds, $buf, MSG_EOR) //
+       my @io = ($y, $err);
+       my $n = $PublicInbox::IPC::send_cmd->($s, \@io, $buf, MSG_EOR) //
                xbail "send: $!";
        my $exp = length($buf);
        $exp == $n or xbail "req @arg sent short ($n != $exp)";