From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <e@80x24.org>
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on dcvr.yhbt.net
X-Spam-Level: 
X-Spam-ASN:  
X-Spam-Status: No, score=-4.2 required=3.0 tests=ALL_TRUSTED,AWL,BAYES_00,
	DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,
	T_SCC_BODY_TEXT_LINE shortcircuit=no autolearn=ham autolearn_force=no
	version=3.4.6
Received: from localhost (dcvr.yhbt.net [127.0.0.1])
	by dcvr.yhbt.net (Postfix) with ESMTP id 49D751F452
	for <meta@public-inbox.org>; Mon, 27 Nov 2023 21:57:14 +0000 (UTC)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=80x24.org;
	s=selector1; t=1701122234;
	bh=KX2P2XuufaPSbjktimx9YSTriC7fEGIgAZemPgR36dk=;
	h=From:To:Subject:Date:In-Reply-To:References:From;
	b=SkWQDiAcmqGTEHEZIvSmxZXf0m/chQj0ZIgIkvdJG14q1SuiHQ6uVO+M/GMccqz3F
	 KSDsx9uXT6ckixlMKYpgEtZWq/rf25bhrnCCyB5AQ3QZlkAz5nr/4AVlIYXUhKLXkb
	 4+Kr92qsPvkRqRmWUDz7vYKFdXQXYhE6AlNgTnLs=
From: Eric Wong <e@80x24.org>
To: meta@public-inbox.org
Subject: [PATCH 1/2] xap_helper: avoid strerror(3) inside signal handler
Date: Mon, 27 Nov 2023 21:54:38 +0000
Message-ID: <20231127215439.91487-2-e@80x24.org>
In-Reply-To: <20231127215439.91487-1-e@80x24.org>
References: <20231127215439.91487-1-e@80x24.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
List-Id: <meta.public-inbox.org>

It's not async-signal-safe and the glibc implementation uses
malloc via asnprintf.  Practically it's not a problem unless the
kernel OOMs and the write(2) fails to the self-pipe.
---
 lib/PublicInbox/xap_helper.h | 29 ++++++++++++-----------------
 1 file changed, 12 insertions(+), 17 deletions(-)

diff --git a/lib/PublicInbox/xap_helper.h b/lib/PublicInbox/xap_helper.h
index b6b517d5..1d8437c9 100644
--- a/lib/PublicInbox/xap_helper.h
+++ b/lib/PublicInbox/xap_helper.h
@@ -980,7 +980,8 @@ static void sigp(int sig) // parent signal handler
 {
 	static const char eagain[] = "signals coming in too fast";
 	static const char bad_sig[] = "BUG: bad sig\n";
-	static const char write_err[] = "BUG: sigp write: ";
+	static const char write_errno[] = "BUG: sigp write (errno)";
+	static const char write_zero[] = "BUG: sigp write wrote zero bytes";
 	char c = 0;
 
 	switch (sig) {
@@ -992,23 +993,17 @@ static void sigp(int sig) // parent signal handler
 		_exit(EXIT_FAILURE);
 	}
 	ssize_t w = write(pipefds[1], &c, 1);
-	if (w == sizeof(c)) return;
-	int e = 0;
-	if (w < 0) {
-		e = errno;
-		if (e == EAGAIN) {
-			write(STDERR_FILENO, eagain, sizeof(eagain) - 1);
-			return;
-		}
+	if (w > 0) return;
+	if (w < 0 && errno == EAGAIN) {
+		write(STDERR_FILENO, eagain, sizeof(eagain) - 1);
+		return;
+	} else if (w == 0) {
+		write(STDERR_FILENO, write_zero, sizeof(write_zero) - 1);
+	} else {
+		// strerror isn't technically async-signal-safe, and
+		// strerrordesc_np+strerrorname_np isn't portable
+		write(STDERR_FILENO, write_errno, sizeof(write_errno) - 1);
 	}
-	struct iovec iov[3];
-	iov[0].iov_base = (void *)write_err;
-	iov[0].iov_len = sizeof(write_err) - 1;
-	iov[1].iov_base = (void *)(e ? strerror(e) : "zero write");
-	iov[1].iov_len = strlen((const char *)iov[1].iov_base);
-	iov[2].iov_base = (void *)"\n";
-	iov[2].iov_len = 1;
-	(void)writev(STDERR_FILENO, iov, MY_ARRAY_SIZE(iov));
 	_exit(EXIT_FAILURE);
 }