Date: Wed, 7 Sep 2005 15:04:05 +0300 From: Andrey Simonenko <simon@comsys.ntu-kpi.kiev.ua> To: FreeBSD-gnats-submit@FreeBSD.org Subject: bin/85830: [patch] pam_exec incorrectly works with vfork() Message-ID: <20050907120405.GB295@pm514-9.comsys.ntu-kpi.kiev.ua> Resent-Message-ID: <200509071210.j87CA7ch080989@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 85830
>Category: bin
>Synopsis: [patch] pam_exec incorrectly works with vfork()
>Confidential: no
>Severity: serious
>Priority: medium
>Responsible: freebsd-bugs
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: change-request
>Submitter-Id: current-users
>Arrival-Date: Wed Sep 07 12:10:07 GMT 2005
>Closed-Date:
>Last-Modified:
>Originator: Andrey Simonenko
>Release: FreeBSD 5.4
>Organization:
>Environment:
>Description:
pam_exec PAM module incorrectly works with vfork() system call.
It uses childerr variable to report from a vfork'ed child to a
parent if execve() in a child failed. There is not guaranty that
a parent will see childerr exactly the same as it was set by its
child, because compiler can optimize the code.
At first time, I decided simply to declare childpid variable as
volatile (and this works), but having read some discussions, I
moved vfork-execve sequence to separate function vfork_execve()
and automatic variable childerr became global volatile variable.
>How-To-Repeat:
>Fix:
diff -ruN pam_exec.orig/pam_exec.c pam_exec/pam_exec.c
--- pam_exec.orig/pam_exec.c Tue Sep 6 23:28:39 2005
+++ pam_exec/pam_exec.c Tue Sep 6 23:58:59 2005
@@ -47,11 +47,26 @@
#include <security/pam_modules.h>
#include <security/openpam.h>
+static volatile int childerr;
+
+static pid_t
+vfork_execve(const char *prog, char *const argv[], char *const envlist[])
+{
+ pid_t pid;
+
+ if ((pid = vfork()) == 0) {
+ execve(prog, argv, envlist);
+ childerr = errno;
+ _exit(1);
+ }
+ return (pid);
+}
+
static int
_pam_exec(pam_handle_t *pamh __unused, int flags __unused,
int argc, const char *argv[])
{
- int childerr, status;
+ int status;
char **env, **envlist;
pid_t pid;
@@ -64,11 +79,7 @@
*/
envlist = pam_getenvlist(pamh);
childerr = 0;
- if ((pid = vfork()) == 0) {
- execve(argv[0], argv, envlist);
- childerr = errno;
- _exit(1);
- }
+ pid = vfork_execve(argv[0], (char *const *)argv, envlist);
for (env = envlist; *env != NULL; ++env)
free(*env);
free(envlist);
@@ -81,7 +92,7 @@
return (PAM_SYSTEM_ERR);
}
if (childerr != 0) {
- openpam_log(PAM_LOG_ERROR, "execv(): %m");
+ openpam_log(PAM_LOG_ERROR, "execve(): %m");
return (PAM_SYSTEM_ERR);
}
if (WIFSIGNALED(status)) {
>Release-Note:
>Audit-Trail:
>Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20050907120405.GB295>