[Pam-developers] pam_limits: Verify that the process found from the utmp record is still alive
Dmitry V. Levin
ldv at altlinux.org
Mon Jan 20 13:18:02 UTC 2014
On Mon, Jan 20, 2014 at 09:26:52AM +0100, Tomas Mraz wrote:
> The utmp entry for an user might be wrong for example when the process
> that should update the entry on logout crashes. This patch by
> Christopher Hailey tries to verify that the process that is associated
> with the utmp record is still alive. Of course this is not perfectly
> race-free but still better than nothing.
> OK to commit?
> --- Linux-PAM-1.0.4/modules/pam_limits/pam_limits.c 2009-02-20 13:27:14.000000000 +0000
> +++ Linux-PAM-1.0.4/modules/pam_limits/pam_limits.c 2014-01-13 23:41:07.143383699 +0000
> @@ -26,10 +26,11 @@
> #include <stdlib.h>
> #include <errno.h>
> #include <syslog.h>
> #include <stdarg.h>
> #include <sys/types.h>
> +#include <signal.h>
> #include <sys/stat.h>
> #include <sys/resource.h>
> #include <limits.h>
> #include <glob.h>
> #include <utmp.h>
> @@ -265,10 +266,14 @@
> || (pl->login_limit_def == LIMITS_DEF_GROUP)
> || (pl->login_limit_def == LIMITS_DEF_DEFAULT))
> && strncmp(name, ut->UT_USER, sizeof(ut->UT_USER)) != 0) {
> continue;
> }
> + if (kill(ut->ut_pid,0)) /* process alive? */
> + pam_syslog(pamh, LOG_WARNING,
> + "User %s in utmp but proc is dead!!",name);
> + continue; /* nope */
> if ((pl->login_limit_def == LIMITS_DEF_ALLGROUP)
> && !pam_modutil_user_in_group_nam_nam(pamh, ut->UT_USER, pl->login_group)) {
> continue;
> }
> }
The patch is awful. First of all, it adds unconditional "continue"
statement, thus rendering the whole utmp check useless.
This is not a python script, is it?
The check for process existence is added in the middle of applicability
checks (after LIMITS_DEF_GROUP but before LIMITS_DEF_ALLGROUP).
It should rather be added after these checks.
Only ESRCH tells that the process with the given pid doesn't exist, other
errno codes do not mean that, so errno == ESRCH should be added to the
check.
The log message is written in a different style that doesn't match the
style of other messages issued by this module. Please change it to
something like "stale utmp entry (user=%s, pid=%d) ignored".
--
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/pam-developers/attachments/20140120/180562cd/attachment.sig>
More information about the Pam-developers
mailing list