From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: Hannes Laimer <h.laimer@proxmox.com>, pve-devel@lists.proxmox.com
Subject: Re: [PATCH pve-access-control 1/1] pam: fork for PAM authentication to isolate SIGCHLD handler
Date: Thu, 12 Mar 2026 10:44:31 +0100 [thread overview]
Message-ID: <1773308530.j96g4rur71.astroid@yuna.none> (raw)
In-Reply-To: <20260304134649.82272-3-h.laimer@proxmox.com>
On March 4, 2026 2:46 pm, Hannes Laimer wrote:
> PAM modules can temporarily override $SIG{CHLD}, causing SIGCHLDs from
> RESTEnvironment worker processes to be lost. Run the PAM interaction in
> a subprocess via PVE::Tools::run_fork to contain any signal handler
> manipulation to the child.
>
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Tested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
this patch alone fixes the reproducer for me, tracing the signals also
clearly shows that the PAM unix_chkpwd children are now decoupled
properly from our worker handling..
> ---
> src/PVE/Auth/PAM.pm | 74 +++++++++++++++++++++++++--------------------
> 1 file changed, 42 insertions(+), 32 deletions(-)
>
> diff --git a/src/PVE/Auth/PAM.pm b/src/PVE/Auth/PAM.pm
> index 3aacfc0..8586da5 100755
> --- a/src/PVE/Auth/PAM.pm
> +++ b/src/PVE/Auth/PAM.pm
> @@ -27,45 +27,55 @@ sub authenticate_user {
> # user (www-data) need to be able to read /etc/passwd /etc/shadow
> die "no password\n" if !$password;
>
> - my $pamh = Authen::PAM->new(
> - 'proxmox-ve-auth',
> - $username,
> - sub {
> - my @res;
> - while (@_) {
> - my $msg_type = shift;
> - my $msg = shift;
> - push @res, (0, $password);
> - }
> - push @res, 0;
> - return @res;
> - },
> - );
> -
> - if (!ref($pamh)) {
> - my $err = $pamh->pam_strerror($pamh);
> - die "error during PAM init: $err";
> + # PAM modules may temporarily override $SIG{CHLD}, causing SIGCHLDs from
> + # RESTEnvironment workers to be lost. Running the PAM interaction in a fork
> + # isolates any such handler manipulation from the parent process.
> + my $client_ip;
> + if (my $rpcenv = PVE::RPCEnvironment::get()) {
> + $client_ip = $rpcenv->get_client_ip();
> }
>
> - if (my $rpcenv = PVE::RPCEnvironment::get()) {
> - if (my $ip = $rpcenv->get_client_ip()) {
> - $pamh->pam_set_item(PAM_RHOST(), $ip);
> + PVE::Tools::run_fork(sub {
> + my $pamh = Authen::PAM->new(
> + 'proxmox-ve-auth',
> + $username,
> + sub {
> + my @res;
> + while (@_) {
> + my $msg_type = shift;
> + my $msg = shift;
> + push @res, (0, $password);
> + }
> + push @res, 0;
> + return @res;
> + },
> + );
> +
> + if (!ref($pamh)) {
> + my $err = $pamh->pam_strerror($pamh);
> + die "error during PAM init: $err";
> }
> - }
>
> - my $res;
> + if ($client_ip) {
> + $pamh->pam_set_item(PAM_RHOST(), $client_ip);
> + }
>
> - if (($res = $pamh->pam_authenticate(0)) != PAM_SUCCESS) {
> - my $err = $pamh->pam_strerror($res);
> - die "$err\n";
> - }
> + my $res;
>
> - if (($res = $pamh->pam_acct_mgmt(0)) != PAM_SUCCESS) {
> - my $err = $pamh->pam_strerror($res);
> - die "$err\n";
> - }
> + if (($res = $pamh->pam_authenticate(0)) != PAM_SUCCESS) {
> + my $err = $pamh->pam_strerror($res);
> + die "$err\n";
> + }
> +
> + if (($res = $pamh->pam_acct_mgmt(0)) != PAM_SUCCESS) {
> + my $err = $pamh->pam_strerror($res);
> + die "$err\n";
> + }
> +
> + $pamh = 0; # call destructor
>
> - $pamh = 0; # call destructor
> + return 1;
> + });
>
> return 1;
> }
> --
> 2.47.3
>
>
>
>
>
>
next prev parent reply other threads:[~2026-03-12 9:45 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-04 13:46 [PATCH access-control/common 0/2] address probblem with SIGCHLD handler being temporarily overwritten Hannes Laimer
2026-03-04 13:46 ` [PATCH pve-common 1/1] RESTEnvironment: periodically reap workers as SIGCHLD fallback Hannes Laimer
2026-03-12 9:48 ` Fabian Grünbichler
2026-03-04 13:46 ` [PATCH pve-access-control 1/1] pam: fork for PAM authentication to isolate SIGCHLD handler Hannes Laimer
2026-03-12 9:44 ` Fabian Grünbichler [this message]
2026-03-06 17:16 ` [PATCH access-control/common 0/2] address probblem with SIGCHLD handler being temporarily overwritten Stefan Hanreich
2026-03-11 14:56 ` Michael Köppl
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1773308530.j96g4rur71.astroid@yuna.none \
--to=f.gruenbichler@proxmox.com \
--cc=h.laimer@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox