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 <[email protected]> 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 <[email protected]> --- 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 <[email protected]> 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
