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?
--
Tomas Mraz
No matter how far down the wrong road you've gone, turn back.
Turkish proverb
(You'll never know whether the road is wrong though.)