On Pá, 2014-08-22 at 17:40 +0200, Tomas Mraz wrote:
On Čt, 2014-08-14 at 13:59 +0200, Tomas Mraz wrote:
On Čt, 2014-08-14 at 08:40 +0200, Robin Hack wrote:
Hi all.
I know that kernel will close opened file handlers on the end of process life... This will make coverity more happy and also it's good habit to cleanup your mess :).
I'd like to have an opinion of other Linux PAM upstream developers whether we want to fix all these cases where we do not close file descriptors before _exit().
Also there is a problem this patch is not correct although the current implementation is disputable. The current implementation uses this file to replace the stdout but this is done by close(STDOUT_FILENO) just before the open() call but there is no guarantee that STDIN is opened at that time, which means the file descriptor might be in reality STDIN_FILENO.
The attached patch fixes it. And I do not think closing STDOUT_FILENO is appropriate as in theory something could be written to it even during the _exit() call.
diff --git a/modules/pam_exec/pam_exec.c b/modules/pam_exec/pam_exec.c index 12c4444..0f18fcc 100644 --- a/modules/pam_exec/pam_exec.c +++ b/modules/pam_exec/pam_exec.c @@ -360,9 +360,20 @@ call_exec (const char *pam_type, pam_handle_t *pamh, logfile); _exit (err); }
if (i != STDOUT_FILENO)
{
if (dup2 (i, STDOUT_FILENO) == -1)
{
int err = errno;
pam_syslog (pamh, LOG_ERR, "dup2 failed: %m");
close (i);
_exit (err);
}
close (i);
if (asprintf (&buffer, "*** %s", ctime (&tm)) > 0) {}
pam_modutil_write (i, buffer, strlen (buffer));
}pam_modutil_write (STDOUT_FILENO, buffer, strlen (buffer)); free (buffer); }
Anybody wants to comment on this patch above, that tries to avoid depending on the stdin in pam_exec being opened?
I will commit the patch above unless anybody has objections as depending on the open() to return STDOUT_FILENO is hacky.