From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-Status: No, score=-4.0 required=3.0 tests=ALL_TRUSTED,BAYES_00 shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id D0C811FC0D for ; Wed, 24 Mar 2021 09:23:36 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 9/9] lei-daemon: do not leak FDs on bogus requests Date: Wed, 24 Mar 2021 14:23:35 +0500 Message-Id: <20210324092335.12345-10-e@80x24.org> In-Reply-To: <20210324092335.12345-1-e@80x24.org> References: <20210324092335.12345-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: If a client passes us the incorrect number of FDs, we'll vivify them into PerlIO objects so they can be auto-closed. Using POSIX::close was considered, but it would've been more code to handle an uncommon case. --- lib/PublicInbox/LEI.pm | 15 +++++++-------- t/lei-daemon.t | 29 +++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm index 878685f1..e5211764 100644 --- a/lib/PublicInbox/LEI.pm +++ b/lib/PublicInbox/LEI.pm @@ -981,17 +981,16 @@ sub accept_dispatch { # Listener {post_accept} callback return send($sock, 'timed out waiting to recv FDs', MSG_EOR); # (4096 * 33) >MAX_ARG_STRLEN my @fds = $recv_cmd->($sock, my $buf, 4096 * 33) or return; # EOF - if (scalar(@fds) == 4) { - for my $i (0..3) { - my $fd = shift(@fds); - open($self->{$i}, '+<&=', $fd) and next; - send($sock, "open(+<&=$fd) (FD=$i): $!", MSG_EOR); - } - } elsif (!defined($fds[0])) { + if (!defined($fds[0])) { warn(my $msg = "recv_cmd failed: $!"); return send($sock, $msg, MSG_EOR); } else { - return; + my $i = 0; + for my $fd (@fds) { + open($self->{$i++}, '+<&=', $fd) and next; + send($sock, "open(+<&=$fd) (FD=$i): $!", MSG_EOR); + } + return if scalar(@fds) != 4; } $self->{2}->autoflush(1); # keep stdout buffered until x_it|DESTROY # $ENV_STR = join('', map { "\0$_=$ENV{$_}" } keys %ENV); diff --git a/t/lei-daemon.t b/t/lei-daemon.t index c30e5ac1..35e059b9 100644 --- a/t/lei-daemon.t +++ b/t/lei-daemon.t @@ -2,8 +2,16 @@ # Copyright (C) 2020-2021 all contributors # License: AGPL-3.0+ use strict; use v5.10.1; use PublicInbox::TestCommon; +use Socket qw(AF_UNIX SOCK_SEQPACKET MSG_EOR pack_sockaddr_un); +use PublicInbox::Spawn qw(which); test_lei({ daemon_only => 1 }, sub { + my $send_cmd = PublicInbox::Spawn->can('send_cmd4') // do { + require PublicInbox::CmdIPC4; + PublicInbox::CmdIPC4->can('send_cmd4'); + }; + $send_cmd or BAIL_OUT 'started testing lei-daemon w/o send_cmd4!'; + my $sock = "$ENV{XDG_RUNTIME_DIR}/lei/5.seq.sock"; my $err_log = "$ENV{XDG_RUNTIME_DIR}/lei/errors.log"; lei_ok('daemon-pid'); @@ -22,6 +30,27 @@ test_lei({ daemon_only => 1 }, sub { is($pid, $pid_again, 'daemon-pid idempotent'); like($lei_err, qr/phail/, 'got mock "phail" error previous run'); + SKIP: { + skip 'only testing open files on Linux', 1 if $^O ne 'linux'; + my $d = "/proc/$pid/fd"; + skip "no $d on Linux" unless -d $d; + 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); + } + lei_ok('daemon-pid'); + chomp($pid = $lei_out); + is($pid, $pid_again, 'pid unchanged after failed reqs'); + my @after = sort(glob("$d/*")); + is_deeply(\@before, \@after, 'open files unchanged') or + diag explain([\@before, \@after]);; + } lei_ok(qw(daemon-kill)); is($lei_out, '', 'no output from daemon-kill'); is($lei_err, '', 'no error from daemon-kill');