linux-kernel - Re: perf script: rwtop: SIGALRM and pipe read race

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20120918093006.GB2467@turtle.usersys.redhat.com>
Date:	Tue, 18 Sep 2012 11:30:07 +0200
From:	Andrew Jones <drjones@...hat.com>
To:	David Ahern <dsahern@...il.com>
Cc:	acme@...stprotocols.net, linux-kernel@...r.kernel.org,
	a.p.zijlstra@...llo.nl, paulus@...ba.org, mingo@...hat.com,
	tzanussi@...il.com
Subject: Re: perf script: rwtop: SIGALRM and pipe read race

On Tue, Sep 18, 2012 at 11:05:42AM +0200, Andrew Jones wrote:
> Please read the link I posted to the Perl documentation. The standard
> Perl signal handling doesn't support SA_RESTART. The Perl developers
> found it could lead to data corruption. The patch above attempts to
> replace the standard signal handler with a safe one that supports
> SA_RESTART. It has nothing to do with the display routine, and
> everything to do with avoiding the need for your EINTR patch.

Oh, I should state that it *does* avoid your EINTR patch. With the -D
added to the record script, then besides the unsigned $ret issue,
rwtop works fine for me now with with this SA_RESTART patch.

Below is a v2 where I've also updated the minimum version of Perl
needed. My understanding is that < 5.8.0 $SIG might work fine.
>= 5.8.0 it won't, and 5.8.2 or later is needed for ->safe().

diff --git a/tools/perf/scripts/perl/rwtop.pl b/tools/perf/scripts/perl/rwtop.pl
index 4bb3ecd..617a4d5 100644
--- a/tools/perf/scripts/perl/rwtop.pl
+++ b/tools/perf/scripts/perl/rwtop.pl
@@ -9,7 +9,7 @@
 # refreshed every [interval] seconds.  The default interval is 3
 # seconds.
 
-use 5.010000;
+use 5.8.2;
 use strict;
 use warnings;
 
@@ -17,6 +17,7 @@ use lib "$ENV{'PERF_EXEC_PATH'}/scripts/perl/Perf-Trace-Util/lib";
 use lib "./Perf-Trace-Util/lib";
 use Perf::Trace::Core;
 use Perf::Trace::Util;
+use POSIX qw/SIGALRM SA_RESTART/;
 
 my $default_interval = 3;
 my $nlines = 20;
@@ -90,7 +91,10 @@ sub syscalls::sys_enter_write
 
 sub trace_begin
 {
-    $SIG{ALRM} = \&set_print_pending;
+    my $sa = POSIX::SigAction->new(\&set_print_pending);
+    $sa->flags(SA_RESTART);
+    $sa->safe(1);
+    POSIX::sigaction(SIGALRM, $sa) or die "Can't set SIGALRM handler: $!\n";
     alarm 1;
 }
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/