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.
--
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.)