From nobody Fri Jun 02 22:51:48 2023 X-Original-To: dev-commits-src-main@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 4QXytD6LzFz4YJYt; Fri, 2 Jun 2023 22:51:48 +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 4QXytD5rvGz4VHL; Fri, 2 Jun 2023 22:51:48 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1685746308; 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=P6ccijv9DVnaJBBu2e2XsWitN+wYnVJZDa5Zeiv0qqk=; b=YUBpXLK5hXOJ1bHs7tUY1no2BkiPJ01TY3Ifgsw6Obze1vd+EPWXOGfj+rCD1kn5CVlQqR FQmbVl85kL1/3jQf21P0K/DVtg4tPzwLhnASvSIlrffPdua9y7VkQbdKpMqTOzRGLfK9Iw Tk9sVv0eA26sm+HqURKsIkOawPoYjmgMSCu9pkuf6qtitEFC+A7Qn61gJHfI+AhlMKRyXq eEf2LnyBmIiQSJ0itOmuCWQn2muyd1t9q4ZhJulClCy7vJucdT9msLCMFB8JAl4/NKPEW4 wbX1/72Xanq2t2kt1+WfJ4n9yHNEKKKa0qBiHWCZzUN49jqG4pfgEl/YzeURYw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1685746308; 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=P6ccijv9DVnaJBBu2e2XsWitN+wYnVJZDa5Zeiv0qqk=; b=mNf98eFrTQ4rQbrcsEcUZiC32e4ltD5am+z+aE1EtSxTMbcESZt0nHyMzs79Wo+q96KJKM rWXzujFQjrWo8u/OIWZ7YaJgVyW+kJsMy8qnv7GYd1e3dXhbY3ORPnjdURy6gxV9THmtkV 1z2zNIzd5XxxHhRhiZQ0xVSspCoI/87ROTC8I9NayMXhSMzr7bbMy3vQeCYIYF6hmNwT4X sSSZ6weIsP55Ya38kwbtBG+ZN1UfWGeGPrsu2bQna7h3ezej4SSkWxwnmhZAx9tkuieb0y IVdcrrIHT7YxgC57kaiZEoloax5NCj/NgMR+x2OPQzP+8xVbq3bI55bbYpdotA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1685746308; a=rsa-sha256; cv=none; b=mmB4eHfrhVIDF8fsxHPANCdQRGT1QYk95P3+5ZYTMLi+6gPo1jjnXh+1xNHw6GwY+oDy7N zm7VvRLmR9xVaQ78UvxdKLTk8+6nhD3CzHDKe2LDbed9OHF7kJ4SYB5nBxVp+aqE6kFs1T R3xVi7GvLJP12zcV47b/RePH4dg8oKUqqGKPVC+BpTJEV0BaHnqVBMUOefeKHb1XPHpwub 0IGNVw+IHyVe9xsoZb5j0o2pwool5TGd6L1Rke5GgUnDb1dEf4rEYmCJG9tdeXguI6Ottr MSwGNEHOAvM5nTbQ68vEBAYPiuF3qGnYvXas1N8sX9GHO6Mw5ty7w++rslh7LQ== 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 4QXytD4gcqz1Gf7; Fri, 2 Jun 2023 22:51:48 +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 352MpmxA017519; Fri, 2 Jun 2023 22:51:48 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 352Mpmug017518; Fri, 2 Jun 2023 22:51:48 GMT (envelope-from git) Date: Fri, 2 Jun 2023 22:51:48 GMT Message-Id: <202306022251.352Mpmug017518@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Konstantin Belousov Subject: git: 8dad5ece4947 - main - dd(1): neutralize SIGINT while non-async-signal safe code is executing List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: kib X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 8dad5ece49479ba6cdcd5bb4c2799bbd61add3e6 Auto-Submitted: auto-generated X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by kib: URL: https://cgit.FreeBSD.org/src/commit/?id=8dad5ece49479ba6cdcd5bb4c2799bbd61add3e6 commit 8dad5ece49479ba6cdcd5bb4c2799bbd61add3e6 Author: Konstantin Belousov AuthorDate: 2023-05-26 10:27:02 +0000 Commit: Konstantin Belousov CommitDate: 2023-06-02 22:06:27 +0000 dd(1): neutralize SIGINT while non-async-signal safe code is executing making the SIGINT handler (the terminate() function) safe to execute at any interruption moment. This fixes a race in 5807f35c541c26bbd91a3ae12506cd8dd8f20688 where SIGINT delivered right after the check_terminate() but before a blocking syscall would not cause abort. Do it by setting the in_io flag around potentially blocking io syscalls. If handler sees the flag, it terminates the program. Otherwise, termination is delegated to the before_io/after_io fences. Reviewed by: Andrew Gierth Sponsored by: The FreeBSD Foundation MFC after: 1 week Differential revision: https://reviews.freebsd.org/D40281 --- bin/dd/dd.c | 22 ++++++++++----------- bin/dd/extern.h | 5 +++-- bin/dd/misc.c | 58 ++++++++++++++++++++++++++++++++++++++++++++----------- bin/dd/position.c | 11 ++++++----- 4 files changed, 67 insertions(+), 29 deletions(-) diff --git a/bin/dd/dd.c b/bin/dd/dd.c index 78a9e8b06720..84d955b235f8 100644 --- a/bin/dd/dd.c +++ b/bin/dd/dd.c @@ -99,8 +99,7 @@ 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); + prepare_io(); (void)setlocale(LC_CTYPE, ""); jcl(argv); @@ -158,9 +157,9 @@ setup(void) iflags = 0; if (ddflags & C_IDIRECT) iflags |= O_DIRECT; - check_terminate(); + before_io(); in.fd = open(in.name, O_RDONLY | iflags, 0); - check_terminate(); + after_io(); if (in.fd == -1) err(1, "%s", in.name); } @@ -197,17 +196,18 @@ setup(void) oflags |= O_FSYNC; if (ddflags & C_ODIRECT) oflags |= O_DIRECT; - check_terminate(); + before_io(); out.fd = open(out.name, O_RDWR | oflags, DEFFILEMODE); - check_terminate(); + after_io(); /* * May not have read access, so try again with write only. * Without read we may have a problem if output also does * not support seeks. */ if (out.fd == -1) { + before_io(); out.fd = open(out.name, O_WRONLY | oflags, DEFFILEMODE); - check_terminate(); + after_io(); out.flags |= NOREAD; cap_rights_clear(&rights, CAP_READ); } @@ -424,9 +424,9 @@ dd_in(void) in.dbrcnt = 0; fill: - check_terminate(); + before_io(); n = read(in.fd, in.dbp + in.dbrcnt, in.dbsz - in.dbrcnt); - check_terminate(); + after_io(); /* EOF */ if (n == 0 && in.dbrcnt == 0) @@ -607,9 +607,9 @@ dd_out(int force) pending = 0; } if (cnt) { - check_terminate(); + before_io(); nw = write(out.fd, outp, cnt); - check_terminate(); + after_io(); out.seek_offset = 0; } else { return; diff --git a/bin/dd/extern.h b/bin/dd/extern.h index e801722560f7..c9de42a152d5 100644 --- a/bin/dd/extern.h +++ b/bin/dd/extern.h @@ -49,8 +49,9 @@ void progress(void); void summary(void); void sigalarm_handler(int); void siginfo_handler(int); -void terminate(int); -void check_terminate(void); +void prepare_io(void); +void before_io(void); +void after_io(void); void unblock(void); void unblock_close(void); diff --git a/bin/dd/misc.c b/bin/dd/misc.c index 5fbea20b7049..c814d926d884 100644 --- a/bin/dd/misc.c +++ b/bin/dd/misc.c @@ -48,6 +48,7 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include #include #include @@ -147,23 +148,58 @@ sigalarm_handler(int signo __unused) need_progress = 1; } -void +static void terminate(int signo) __dead2; +static void terminate(int signo) { - kill_signal = signo; + summary(); + (void)fflush(stderr); + raise(kill_signal); + /* NOT REACHED */ + _exit(1); +} + +static sig_atomic_t in_io = 0; +static sig_atomic_t sigint_seen = 0; + +static void +sigint_handler(int signo __unused) +{ + atomic_signal_fence(memory_order_acquire); + if (in_io) + terminate(SIGINT); + sigint_seen = 1; +} + +void +prepare_io(void) +{ + struct sigaction sa; + int error; + + memset(&sa, 0, sizeof(sa)); + sa.sa_flags = SA_NODEFER | SA_RESETHAND; + sa.sa_handler = sigint_handler; + error = sigaction(SIGINT, &sa, 0); + if (error != 0) + err(1, "sigaction"); } void -check_terminate(void) +before_io(void) { + in_io = 1; + atomic_signal_fence(memory_order_seq_cst); + if (sigint_seen) + terminate(SIGINT); +} - if (kill_signal) { - summary(); - (void)fflush(stderr); - signal(kill_signal, SIG_DFL); - raise(kill_signal); - /* NOT REACHED */ - _exit(128 + kill_signal); - } +void +after_io(void) +{ + in_io = 0; + atomic_signal_fence(memory_order_seq_cst); + if (sigint_seen) + terminate(SIGINT); } diff --git a/bin/dd/position.c b/bin/dd/position.c index a7dd733f0bae..6cb6643982dc 100644 --- a/bin/dd/position.c +++ b/bin/dd/position.c @@ -191,10 +191,11 @@ pos_out(void) /* Read it. */ for (cnt = 0; cnt < out.offset; ++cnt) { - check_terminate(); - if ((n = read(out.fd, out.db, out.dbsz)) > 0) + before_io(); + n = read(out.fd, out.db, out.dbsz); + after_io(); + if (n > 0) continue; - check_terminate(); if (n == -1) err(1, "%s", out.name); @@ -209,9 +210,9 @@ pos_out(void) err(1, "%s", out.name); while (cnt++ < out.offset) { - check_terminate(); + before_io(); n = write(out.fd, out.db, out.dbsz); - check_terminate(); + after_io(); if (n == -1) err(1, "%s", out.name); if (n != out.dbsz)