[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