From nobody Fri May 05 12:45:23 2023 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4QCVlS57cSz49Hk6; Fri, 5 May 2023 12:45:24 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4QCVlS2lj6z3KgL; Fri, 5 May 2023 12:45:23 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1683290724; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=5d5fzhIneASAWce+XB4+/LWm8h7MT93akoXnqeqJYE0=; b=bZaLocqT9c52HT6d0A0Xfm97BFkosX+DzFPlzeZTK4Bq3KFZQjunkbYngcIv+fle5Q55Yz EzGNL3NF243OvqNGLBvF+Dpovdckeuo5503HNNMwt8o85Bcn9kHzzvcctsqHjafg19t2/j fJEAB3tmI1P7ETTqSQygrX8buvdQXYrOYJ5tu/9lrCOZGarldfAxmlioBBmuojY0pxItyh PRErukr+oJI0sKk25JzX9IaQyiowvIOaR0Cn5icz/LJjbVPCKdoCVM+0NN/Ru10T/3umdj FNgL1SJaF/MzARhQ/qakBXwVJyJcihj+Bhd4CoW5lopelWE7HygpoleFjbYC5w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1683290724; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=5d5fzhIneASAWce+XB4+/LWm8h7MT93akoXnqeqJYE0=; b=lY7nxehXBhTT0nTdt1W06GabcE9/nSqZZKZmGtruxYqLerMzbcB5YhsslAbslIwFL9UqoN eXZTrQrry60VVXVYyXbBOaC8tm3cLz80NH5PaIXQAgMsQSkeXuLBrgAI+NO5Wxox3CvYKz Pk9S+0jVYJrktIbGL57BPATFpbJG7apNRN/6Zxf+xCw2oGrjAE8tOAXsgYFur8XTIVWXab y4g9S/WqYj0miuyDXTwqNna6I57gw4f1qM5Usn7EEhgc+i5KcANEzWO0g0HrKnr1ySrp19 pFseMgAbRv+tm18j/C6n4+b8zPIL/mDLHEYipG8yYd35IHWMuHqckUT2VEnO6g== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1683290724; a=rsa-sha256; cv=none; b=bTdZZak95l/8V2WrhpWpVe8xedDB+bC54iF7cxfa4KLu1ssztOG8iCL69/NktHVptdnmia K9wsqB5kSN0Zc4ol0k0r6s8bkjeMPG48S453ltHl+GAWqDHuyqrvceNKDSJK06ymT/I9dU DhaKbtNSY4wcSf9LgVdSCs7YxJcXzWK/qQeh9atGJE8lhMwjt5xbwJMBISPL3XgVZ+ZagW B1wlA/n8H4nrpgNQsFg+YOJIoIoCFjjbqhrhp4i9Fq8OZ1cpQBzzq5in6Sv5moiPy2I4SJ IxXARsJAdvy4F+C2ozYlSKOEmkmf37J+9FpQnQeG5hbFmeejgT28yJuIPxezbg== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4QCVlR31rfzvq5; Fri, 5 May 2023 12:45:23 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 345CjNsH048276; Fri, 5 May 2023 12:45:23 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 345CjNLE048275; Fri, 5 May 2023 12:45:23 GMT (envelope-from git) Date: Fri, 5 May 2023 12:45:23 GMT Message-Id: <202305051245.345CjNLE048275@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: =?utf-8?Q?Dag-Erling=20Sm=C3=B8rgrav?= Subject: git: 5807f35c541c - main - dd: Fix SIGINT handling. List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: des X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 5807f35c541c26bbd91a3ae12506cd8dd8f20688 Auto-Submitted: auto-generated X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by des: URL: https://cgit.FreeBSD.org/src/commit/?id=5807f35c541c26bbd91a3ae12506cd8dd8f20688 commit 5807f35c541c26bbd91a3ae12506cd8dd8f20688 Author: Dag-Erling Smørgrav AuthorDate: 2023-05-05 11:17:44 +0000 Commit: Dag-Erling Smørgrav CommitDate: 2023-05-05 12:42:32 +0000 dd: Fix SIGINT handling. Currently, we handle SIGINT by calling summary() and _exit() directly from the signal handler, which we install after setup(). There are several issues with this: * summary() is not signal safe; * the parent is not informed about the signal; * setup() can block on open(), and catching SIGINT at that stage will produce the correct exit status but will not print anything to stderr as POSIX demands. Fix this by making SIGINT non-restartable, changing our signal handler to only set a flag, installing it before setup(), and checking the termination flag before and after every blocking operation, i.e. open(), read(), write(). Also add two test cases, one for catching SIGINT while opening the input and one for catching it while reading. I couldn't think of an easy way to test catching SIGINT while writing (it's certainly feasible, but perhaps not from a shell script). MFC after: 1 week Sponsored by: Klara, Inc. Reviewed by: cracauer, ngie, imp Differential Revision: https://reviews.freebsd.org/D39641 --- bin/dd/dd.c | 15 ++++++++++++++- bin/dd/extern.h | 2 ++ bin/dd/misc.c | 20 ++++++++++++++++---- bin/dd/position.c | 5 ++++- bin/dd/tests/dd2_test.sh | 44 ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 80 insertions(+), 6 deletions(-) diff --git a/bin/dd/dd.c b/bin/dd/dd.c index c43645fa0073..78a9e8b06720 100644 --- a/bin/dd/dd.c +++ b/bin/dd/dd.c @@ -64,6 +64,7 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include #include #include @@ -91,12 +92,16 @@ char fill_char; /* Character to fill with if defined */ size_t speed = 0; /* maximum speed, in bytes per second */ volatile sig_atomic_t need_summary; volatile sig_atomic_t need_progress; +volatile sig_atomic_t kill_signal; int main(int argc __unused, char *argv[]) { struct itimerval itv = { { 1, 0 }, { 1, 0 } }; /* SIGALARM every second, if needed */ + (void)siginterrupt(SIGINT, 1); + (void)signal(SIGINT, terminate); + (void)setlocale(LC_CTYPE, ""); jcl(argv); setup(); @@ -110,7 +115,6 @@ main(int argc __unused, char *argv[]) (void)signal(SIGALRM, sigalarm_handler); setitimer(ITIMER_REAL, &itv, NULL); } - (void)signal(SIGINT, terminate); atexit(summary); @@ -154,7 +158,9 @@ setup(void) iflags = 0; if (ddflags & C_IDIRECT) iflags |= O_DIRECT; + check_terminate(); in.fd = open(in.name, O_RDONLY | iflags, 0); + check_terminate(); if (in.fd == -1) err(1, "%s", in.name); } @@ -191,7 +197,9 @@ setup(void) oflags |= O_FSYNC; if (ddflags & C_ODIRECT) oflags |= O_DIRECT; + check_terminate(); out.fd = open(out.name, O_RDWR | oflags, DEFFILEMODE); + check_terminate(); /* * May not have read access, so try again with write only. * Without read we may have a problem if output also does @@ -199,6 +207,7 @@ setup(void) */ if (out.fd == -1) { out.fd = open(out.name, O_WRONLY | oflags, DEFFILEMODE); + check_terminate(); out.flags |= NOREAD; cap_rights_clear(&rights, CAP_READ); } @@ -415,7 +424,9 @@ dd_in(void) in.dbrcnt = 0; fill: + check_terminate(); n = read(in.fd, in.dbp + in.dbrcnt, in.dbsz - in.dbrcnt); + check_terminate(); /* EOF */ if (n == 0 && in.dbrcnt == 0) @@ -596,7 +607,9 @@ dd_out(int force) pending = 0; } if (cnt) { + check_terminate(); nw = write(out.fd, outp, cnt); + check_terminate(); out.seek_offset = 0; } else { return; diff --git a/bin/dd/extern.h b/bin/dd/extern.h index 07c08e2ef8f2..e801722560f7 100644 --- a/bin/dd/extern.h +++ b/bin/dd/extern.h @@ -50,6 +50,7 @@ void summary(void); void sigalarm_handler(int); void siginfo_handler(int); void terminate(int); +void check_terminate(void); void unblock(void); void unblock_close(void); @@ -69,3 +70,4 @@ extern u_char casetab[]; extern char fill_char; extern volatile sig_atomic_t need_summary; extern volatile sig_atomic_t need_progress; +extern volatile sig_atomic_t kill_signal; diff --git a/bin/dd/misc.c b/bin/dd/misc.c index 405448eb1cb0..5fbea20b7049 100644 --- a/bin/dd/misc.c +++ b/bin/dd/misc.c @@ -147,11 +147,23 @@ sigalarm_handler(int signo __unused) need_progress = 1; } -/* ARGSUSED */ void -terminate(int sig) +terminate(int signo) { - summary(); - _exit(sig == 0 ? 0 : 1); + kill_signal = signo; +} + +void +check_terminate(void) +{ + + if (kill_signal) { + summary(); + (void)fflush(stderr); + signal(kill_signal, SIG_DFL); + raise(kill_signal); + /* NOT REACHED */ + _exit(128 + kill_signal); + } } diff --git a/bin/dd/position.c b/bin/dd/position.c index 21a100cd3b24..a7dd733f0bae 100644 --- a/bin/dd/position.c +++ b/bin/dd/position.c @@ -191,9 +191,10 @@ pos_out(void) /* Read it. */ for (cnt = 0; cnt < out.offset; ++cnt) { + check_terminate(); if ((n = read(out.fd, out.db, out.dbsz)) > 0) continue; - + check_terminate(); if (n == -1) err(1, "%s", out.name); @@ -208,7 +209,9 @@ pos_out(void) err(1, "%s", out.name); while (cnt++ < out.offset) { + check_terminate(); n = write(out.fd, out.db, out.dbsz); + check_terminate(); if (n == -1) err(1, "%s", out.name); if (n != out.dbsz) diff --git a/bin/dd/tests/dd2_test.sh b/bin/dd/tests/dd2_test.sh index 7632de3bde22..c4a3065763cd 100755 --- a/bin/dd/tests/dd2_test.sh +++ b/bin/dd/tests/dd2_test.sh @@ -1,5 +1,6 @@ # # Copyright (c) 2017 Spectra Logic Corporation +# Copyright (c) 2023 Klara, Inc. # # SPDX-License-Identifier: BSD-2-Clause # @@ -46,8 +47,51 @@ seek_overflow_body() dd if=f.in of=f.out bs=4096 seek=-1 } +atf_test_case sigint +sigint_open_head() +{ + atf_set "descr" "SIGINT while opening destination" +} +sigint_open_body() +{ + atf_check mkfifo fifo + set -m + dd if=fifo of=/dev/null 2>stderr & + pid=$! + sleep 3 + kill -INT $pid + wait $pid + rv=$? + atf_check test "$rv" -gt 128 + atf_check -o inline:"INT\n" kill -l $((rv-128)) + atf_check test -s stderr +} + +atf_test_case sigint +sigint_read_head() +{ + atf_set "descr" "SIGINT while reading source" +} +sigint_read_body() +{ + atf_check mkfifo fifo + (sleep 30 >fifo &) # ensures that dd does not block on open + set -m + dd if=fifo of=/dev/null 2>stderr & + pid=$! + sleep 3 + kill -INT $pid + wait $pid + rv=$? + atf_check test "$rv" -gt 128 + atf_check -o inline:"INT\n" kill -l $((rv-128)) + atf_check test -s stderr +} + atf_init_test_cases() { atf_add_test_case max_seek atf_add_test_case seek_overflow + atf_add_test_case sigint_open + atf_add_test_case sigint_read }