From caaf5e3831956e893d3353b9782c7f86e0dcb00a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= <mail@3v1n0.net>
Date: Mon, 3 Mar 2025 20:25:03 +0100
Subject: [PATCH] pam/go-exec/module: More consistent behavior for fatal
 signals

Sadly some signals such as SIGABRT or SIGSEGV are handled by go and in
the wrong way because it never redirects them as expected, so in such
cases we just fail with a normal exit error instead of because of a
signal.

Reported this upstream and adding comments about.

See: https://github.com/golang/go/issues/72084
---
 pam/go-exec/module.c               | 11 +++++++++++
 pam/integration-tests/exec_test.go | 10 +++++++++-
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/pam/go-exec/module.c b/pam/go-exec/module.c
index fed1da310..fb48addba 100644
--- a/pam/go-exec/module.c
+++ b/pam/go-exec/module.c
@@ -436,6 +436,17 @@ wait_child_thread (gpointer data)
 
       if (ret == child_pid && WIFEXITED (status))
         {
+          /* Sadly go childs that exits because of SIGABRT or SIGSEGV do not
+           * have a WIFSIGNALED status, but instead exit with 2 exit status.
+           * See: https://pkg.go.dev/runtime
+           * So in such case we just return a generic system error, to be
+           * consistent with signals (plus, we never return pam.ErrSymbol).
+           * This is an upstream bug, but they refuse to fix or allow a
+           * better handling: https://github.com/golang/go/issues/72084
+           */
+          if (WEXITSTATUS (status) == 2)
+            break;
+
           exit_status = WEXITSTATUS (status);
           break;
         }
diff --git a/pam/integration-tests/exec_test.go b/pam/integration-tests/exec_test.go
index 3edb78b7e..68e7288ee 100644
--- a/pam/integration-tests/exec_test.go
+++ b/pam/integration-tests/exec_test.go
@@ -226,7 +226,7 @@ func TestExecModule(t *testing.T) {
 		},
 		"Error_when_client_fails_panicking": {
 			methodCalls: []cliMethodCall{{m: "SimulateClientPanic", args: []any{"Client panicked! (As expected)"}}},
-			wantError:   pam.ErrSymbol,
+			wantError:   pam.ErrSystem,
 		},
 		"Error_when_client_fails_because_an_unhandled_error": {
 			methodCalls: []cliMethodCall{{m: "SimulateClientError", args: []any{"Client error!"}}},
@@ -240,6 +240,14 @@ func TestExecModule(t *testing.T) {
 			methodCalls: []cliMethodCall{{m: "SimulateClientSignal", args: []any{syscall.SIGKILL}}},
 			wantError:   pam.ErrSystem,
 		},
+		"Error_when_client_fails_because_a_client_SIGSEGV_signal": {
+			methodCalls: []cliMethodCall{{m: "SimulateClientSignal", args: []any{syscall.SIGSEGV}}},
+			wantError:   pam.ErrSystem,
+		},
+		"Error_when_client_fails_because_a_client_SIGABRT_signal": {
+			methodCalls: []cliMethodCall{{m: "SimulateClientSignal", args: []any{syscall.SIGABRT}}},
+			wantError:   pam.ErrSystem,
+		},
 	}
 	for name, tc := range cliTests {
 		t.Run("Client "+name, func(t *testing.T) {