michael/webmin
1
0
Fork 0
mirror of https://github.com/webmin/webmin.git synced 2026-06-28 06:50:25 +01:00
Code Issues Actions 3 Packages Projects Releases Wiki Activity

Fix possible startup loop with stale PID file after PID reuse

Browse Source 
This PR fixes an issue where, after an unclean exit, Webmin can leave `miniserv.pid` behind.

If the kernel later reuses that PID for an unrelated process, the startup guard only checked that the PID was alive and refused to start with “Webmin is already running”.

With systemd restart handling, this can leave Webmin permanently down until the PID file is manually removed.

This change verifies that the live PID actually belongs to `miniserv.pl` running the same config before treating it as an active Webmin instance.

On Linux, it reads `/proc/<pid>/cmdline`, checks the miniserv script, and compares the config file by inode so symlinked paths still match and Usermin is correctly distinguished.

If the PID is confirmed unrelated, the stale PID file is removed and startup continues. If the process cannot be inspected, the previous conservative behavior is preserved.

Also hardens PID-file parsing with chomp and numeric validation, and adds tests for unrelated PID reuse, matching config, symlinked config, different miniserv config, and unreadable command-line fallback.
This commit is contained in:
Ilia Ross
2026-06-24 17:37:02 +02:00
parent d2f9e339f2
commit 18bf94af6a
2 changed files with 126 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

65
miniserv.pl
View File

@@ -143,9 +143,18 @@ if (-l $perl_path) {
if (open(PIDFILE, $config{'pidfile'})) {
my $already = <PIDFILE>;
close(PIDFILE);
chop($already);
if ($already && $already != $$ && kill(0, $already)) {
die "Webmin is already running with PID $already\n";
chomp($already);
$already =~ s/^\s+|\s+$//g;
if ($already =~ /^\d+$/ && $already != $$ && kill(0, $already)) {
my $active = &pid_is_miniserv_for_config($already, $config_file);
if (defined($active) && !$active) {
warn "Ignoring stale Webmin PID file $config{'pidfile'} ".
"for unrelated PID $already\n";
unlink($config{'pidfile'});
}
else {
die "Webmin is already running with PID $already\n";
}
}
}
@@ -5182,6 +5191,56 @@ close($fh);
return $rv;
}
# read_process_cmdline(pid)
# Returns a process command line from /proc, or undef if unavailable.
sub read_process_cmdline
{
my ($pid) = @_;
return undef if (!$pid || $pid !~ /^\d+$/);
return &read_any_file("/proc/$pid/cmdline");
}
# same_config_file(file1, file2)
# Returns 1 if two config paths refer to the same file.
sub same_config_file
{
my ($file1, $file2) = @_;
return 0 if (!$file1 || !$file2);
return 1 if ($file1 eq $file2);
my @stat1 = stat($file1);
my @stat2 = stat($file2);
return @stat1 && @stat2 && $stat1[0] == $stat2[0] &&
$stat1[1] == $stat2[1];
}
# pid_is_miniserv_for_config(pid, config-file)
# Returns 1 if pid appears to be miniserv for this config, 0 if it is a
# different process, or undef if this platform cannot tell.
sub pid_is_miniserv_for_config
{
my ($pid, $config_file) = @_;
my $cmdline = &read_process_cmdline($pid);
return undef if (!defined($cmdline));
return 0 if ($cmdline eq "");
my @args = split(/\0/, $cmdline);
my $script_idx;
for(my $i=0; $i<@args; $i++) {
if ($args[$i] =~ /(?:^|\/)miniserv\.pl$/) {
$script_idx = $i;
last;
}
}
return 0 if (!defined($script_idx));
my $config_arg;
for(my $i=$script_idx+1; $i<@args; $i++) {
next if ($args[$i] =~ /^-/);
$config_arg = $args[$i];
last;
}
return defined($config_arg) ?
(&same_config_file($config_arg, $config_file) ? 1 : 0) : undef;
}
# update_vital_config()
# Updates %config with defaults, and dies if something vital is missing
sub update_vital_config

64
t/miniserv.t
View File

@@ -1282,6 +1282,70 @@ subtest 'read_any_file' => sub {
'open failure returns undef');
};
# pid_is_miniserv_for_config — avoids stale pidfiles with reused PIDs
subtest 'pid_is_miniserv_for_config' => sub {
my $config = '/etc/webmin/miniserv.conf';
my $pid = $$;
{
no warnings 'redefine';
local *miniserv::read_process_cmdline = sub {
return join("\0", '/usr/sbin/dovecot', '-F');
};
is(miniserv::pid_is_miniserv_for_config($pid, $config), 0,
'live unrelated PID is not treated as active miniserv');
}
{
no warnings 'redefine';
local *miniserv::read_process_cmdline = sub {
return join("\0", '/usr/bin/perl',
'/usr/libexec/webmin/miniserv.pl',
'--nofork', $config);
};
is(miniserv::pid_is_miniserv_for_config($pid, $config), 1,
'matching miniserv config is treated as active');
}
{
require File::Temp;
my ($fh, $real_config) = File::Temp::tempfile(UNLINK => 1);
close($fh);
my $link_config = "$real_config.link";
SKIP: {
symlink($real_config, $link_config) ||
skip 'symlinks are unavailable', 1;
no warnings 'redefine';
local *miniserv::read_process_cmdline = sub {
return join("\0", '/usr/bin/perl',
'/usr/libexec/webmin/miniserv.pl',
$link_config);
};
is(miniserv::pid_is_miniserv_for_config($pid, $real_config), 1,
'symlinked config path is matched by inode');
unlink($link_config);
}
}
{
no warnings 'redefine';
local *miniserv::read_process_cmdline = sub {
return join("\0", '/usr/bin/perl',
'/usr/libexec/usermin/miniserv.pl',
'--nofork', '/etc/usermin/miniserv.conf');
};
is(miniserv::pid_is_miniserv_for_config($pid, $config), 0,
'different miniserv config is not treated as active');
}
{
no warnings 'redefine';
local *miniserv::read_process_cmdline = sub { return undef; };
is(miniserv::pid_is_miniserv_for_config($pid, $config), undef,
'unreadable process command line keeps conservative behavior');
}
};
# read_mime_types — populates %mime from file + addtype_* config
subtest 'read_mime_types' => sub {
no warnings 'once';