Skip to content

Commit

Permalink
pam/go-exec/module: More consistent behavior for fatal signals
Browse files Browse the repository at this point in the history
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: golang/go#72084
  • Loading branch information
3v1n0 committed Mar 3, 2025
1 parent a1cb439 commit 8562672
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 1 deletion.
11 changes: 11 additions & 0 deletions pam/go-exec/module.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
10 changes: 9 additions & 1 deletion pam/integration-tests/exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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!"}}},
Expand All @@ -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) {
Expand Down

0 comments on commit 8562672

Please sign in to comment.