[lintian] 02/02: Introduce a do_fork() sub to clear signal handlers
This is an automated email from the git hooks/post-receive script.
nthykier pushed a commit to branch master
in repository lintian.
commit 507dabf93a86208f75a2b9095aa22b596886985f
Author: Niels Thykier <niels@thykier.net>
Date: Tue Oct 11 18:05:21 2016 +0000
Introduce a do_fork() sub to clear signal handlers
TL;DR: Without this Lintian turns into to a fork-bomb with an
ill-timed SIGINT/SIGTERM.
Lintian delegates all of the unpacking to the L::Unpacker module.
This module basically forks and lets the subprocess do an unpacking
job. At a quick review of the code, it would seem that the subprocess
cannot "escape" from its scope and it would always either call
"POSIX::_exit" or "exec".
Nonetheless, there is a subtle race-condition post-forking if Lintian
would receive a SIGINT/SIGTERM. The issue comes from the fact that
the subprocess inherits the signal handler of the parent process,
which uses a "print + die" to throw an exception[1]. Thus, in reality
*any* line in the child process can throw an exception (via a signal).
For an end user, this is generally not terribly visible. When the
child escapes out, it will still cause the unpacking to fail.
Accordingly, the current package will not be checked and at worst the
clean up may disturb a currently running subprocess.
However, when lintian is processing multiple groups, the child now
escapes and starts the next group. Which means we now have at least
*two* processes trying to do unpacking. This is an issue that is
further agitated by reporting-lintian-harness now sending SIGTERM to
lintian, when it has not terminated within its 4 hour deadline.
This patch aims to solve all of that by introducing a new "do_fork"
sub to replace all uses of "fork()". This sub will mask all signals,
fork, drop the signal handlers in the child and then restore the
original signalmask.
[1] This can be perceived locally by running lintian and trying to
interrupt it during unpacking (use --debug to see when it starts). A
successful trigger will cause lintian to print *multiple*
"N: Interrupted" lines. The excess lines comes from child
processes.
Signed-off-by: Niels Thykier <niels@thykier.net>
---
checks/manpages.pm | 4 ++--
checks/scripts.pm | 4 ++--
debian/changelog | 9 +++++++++
lib/Lintian/Command/Simple.pm | 6 ++++--
lib/Lintian/Unpacker.pm | 4 ++--
lib/Lintian/Util.pm | 34 +++++++++++++++++++++++++++++++++-
lib/Test/Lintian/Harness.pm | 4 ++--
7 files changed, 54 insertions(+), 11 deletions(-)
diff --git a/checks/manpages.pm b/checks/manpages.pm
index 619a165..5838bba 100644
--- a/checks/manpages.pm
+++ b/checks/manpages.pm
@@ -31,7 +31,7 @@ use Text::ParseWords ();
use Lintian::Check qw(check_spelling spelling_tag_emitter);
use Lintian::Tags qw(tag);
-use Lintian::Util qw(clean_env drain_pipe fail open_gz);
+use Lintian::Util qw(clean_env do_fork drain_pipe fail open_gz);
sub run {
my (undef, undef, $info, $proc, $group) = @_;
@@ -251,7 +251,7 @@ sub run {
}
my ($read, $write);
pipe($read, $write);
- my $pid = fork();
+ my $pid = do_fork();
if ($pid == 0) {
clean_env;
close STDOUT;
diff --git a/checks/scripts.pm b/checks/scripts.pm
index 5959e95..f40fddd 100644
--- a/checks/scripts.pm
+++ b/checks/scripts.pm
@@ -33,7 +33,7 @@ use Lintian::Check qw($known_shells_regex);
use Lintian::Data;
use Lintian::Relation;
use Lintian::Tags qw(tag);
-use Lintian::Util qw(fail strip);
+use Lintian::Util qw(do_fork fail strip);
# This is a map of all known interpreters. The key is the interpreter
# name (the binary invoked on the #! line). The value is an anonymous
@@ -1212,7 +1212,7 @@ sub script_is_evil_and_wrong {
sub check_script_syntax {
my ($interpreter, $path) = @_;
my $fs_path = $path->fs_path;
- my $pid = fork();
+ my $pid = do_fork();
if ($pid == 0) {
open(STDOUT, '>', '/dev/null');
open(STDERR, '>&', \*STDOUT);
diff --git a/debian/changelog b/debian/changelog
index 5c07a55..40d0f18 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -11,6 +11,15 @@ lintian (2.5.49) UNRELEASED; urgency=medium
* data/spelling/corrections:
+ [JW, PW] Add more corrections.
+ * lib/Lintian/Unpacker.pm:
+ + [NT] Use the new "do_fork()" sub to ensure works do not inherit
+ the default signal handler, which could allow any number of workers
+ to promote themselves to independent "masters" - effectively
+ creating a fork-bomb with an ill-timed signal.
+ * lib/Lintian/Util.pm:
+ + [NT] Add a "do_fork()" sub to ensure signal handling is
+ reset for child processes.
+
-- Chris Lamb <lamby@debian.org> Thu, 06 Oct 2016 19:04:23 +0100
lintian (2.5.48) unstable; urgency=low
diff --git a/lib/Lintian/Command/Simple.pm b/lib/Lintian/Command/Simple.pm
index c9507c7..bdeb45b 100644
--- a/lib/Lintian/Command/Simple.pm
+++ b/lib/Lintian/Command/Simple.pm
@@ -22,6 +22,8 @@ use autodie qw(open close chdir);
use Exporter qw(import);
use POSIX qw(:sys_wait_h);
+use Lintian::Util qw(do_fork);
+
our @EXPORT_OK = qw(rundir background wait_any kill_all);
=head1 NAME
@@ -65,7 +67,7 @@ sub rundir {
my $pid;
my $res;
- $pid = fork();
+ $pid = do_fork();
if (not defined($pid)) {
# failed
$res = -1;
@@ -97,7 +99,7 @@ calling wait() to reap the previous command.
=cut
sub background {
- my $pid = fork();
+ my $pid = do_fork();
if (not defined($pid)) {
# failed
diff --git a/lib/Lintian/Unpacker.pm b/lib/Lintian/Unpacker.pm
index 5b50a8a..9a9ae43 100644
--- a/lib/Lintian/Unpacker.pm
+++ b/lib/Lintian/Unpacker.pm
@@ -26,7 +26,7 @@ use parent 'Class::Accessor::Fast';
use POSIX;
use Lintian::Command::Simple qw(wait_any kill_all);
-use Lintian::Util qw(fail);
+use Lintian::Util qw(do_fork fail);
=head1 NAME
@@ -401,7 +401,7 @@ sub process_tasks {
# collect info
$cmap->select($coll);
$wlist->{'changed'} = 1;
- my $pid = fork//-1;
+ my $pid = do_fork()//-1;
if (not $pid) {
# child
my $ret = 0;
diff --git a/lib/Lintian/Util.pm b/lib/Lintian/Util.pm
index b06f159..f8ee546 100644
--- a/lib/Lintian/Util.pm
+++ b/lib/Lintian/Util.pm
@@ -28,6 +28,7 @@ use Carp qw(croak);
use Cwd qw(abs_path);
use Errno qw(ENOENT);
use Exporter qw(import);
+use POSIX qw(sigprocmask SIG_BLOCK SIG_UNBLOCK SIG_SETMASK);
use constant {
DCTRL_DEBCONF_TEMPLATE => 1,
@@ -64,6 +65,7 @@ BEGIN {
file_is_encoded_in_non_utf8
is_string_utf8_encoded
fail
+ do_fork
strip
lstrip
rstrip
@@ -879,6 +881,36 @@ sub file_is_encoded_in_non_utf8 {
return $line;
}
+=item do_fork()
+
+Overrides fork to reset signal handlers etc. in the child.
+
+=cut
+
+sub do_fork() {
+ my ($pid, $fork_error);
+ my $orig_mask = POSIX::SigSet->new;
+ my $fork_mask = POSIX::SigSet->new;
+ $fork_mask->fillset;
+ sigprocmask(SIG_BLOCK, $fork_mask, $orig_mask)
+ or die("sigprocmask failed: $!\n");
+ $pid = CORE::fork();
+ $fork_error = $!;
+ if ($pid == 0) {
+ for my $sig (keys(%SIG)) {
+ if (ref($SIG{$sig}) eq 'CODE') {
+ $SIG{$sig} = 'DEFAULT';
+ }
+ }
+ }
+ sigprocmask(SIG_SETMASK, $orig_mask, undef)
+ or die("sigprocmask failed: $!\n");
+ if ($pid == -1) {
+ $! = $fork_error;
+ }
+ return $pid;
+}
+
=item system_env (CMD)
Behaves like system (CMD) except that the environment of CMD is
@@ -887,7 +919,7 @@ cleaned (as defined by L</clean_env>(1)).
=cut
sub system_env {
- my $pid = fork;
+ my $pid = do_fork;
if (not defined $pid) {
return -1;
} elsif ($pid == 0) {
diff --git a/lib/Test/Lintian/Harness.pm b/lib/Test/Lintian/Harness.pm
index c812edb..861c686 100644
--- a/lib/Test/Lintian/Harness.pm
+++ b/lib/Test/Lintian/Harness.pm
@@ -49,7 +49,7 @@ use File::Temp qw(tempfile);
use POSIX qw(ENOENT);
use Text::Template;
-use Lintian::Util qw(fail read_dpkg_control slurp_entire_file);
+use Lintian::Util qw(do_fork fail read_dpkg_control slurp_entire_file);
our @EXPORT_OK = qw(
chdir_runcmd
@@ -298,7 +298,7 @@ Returns 0 on success and non-zero otherwise.
sub chdir_runcmd {
my ($dir, $cmd, $log) = @_;
- my $pid = fork();
+ my $pid = do_fork();
if ($pid) {
waitpid $pid, 0;
return $?;
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/lintian/lintian.git
Reply to: