On Fri, Jan 17, 2020 at 04:06:15PM +0100, Tomas Mraz wrote:
The pam_modutil_sanitize_fd() when called to provide a dummy pipes
for
stdout and stderr closes the read end of the newly created dummy pipe.
This is wrong as any writes to such pipe will trigger SIGPIPE. The
original code did a trick to avoid that by dup2-ing the read end of the
pipe as stdin. However it does not necessarily have to be stdin, we
just need to ensure the read end of the pipe is not closed.
I didn't want to leave the read end of the pipe open, but
SIGPIPE is not a price I'm ready to pay for that.
The attached patch implements the fix. OK to commit?
There are some technical issues with the patch.
[...]
diff --git a/libpam/pam_modutil_sanitize.c
b/libpam/pam_modutil_sanitize.c
index 605c859..bad41eb 100644
--- a/libpam/pam_modutil_sanitize.c
+++ b/libpam/pam_modutil_sanitize.c
@@ -60,7 +60,16 @@ redirect_out_pipe(pam_handle_t *pamh, int fd, const char *name)
The function has a comment saying that it
"Creates a pipe, closes its read end, redirects fd to its write end".
If the contract is changed, the comment has to be updated.
return -1;
}
- close(out[0]);
+ /* try to keep the read end open */
I think this function can either succeed or fail,
it shall not leave the caller in an incomplete state.
+ if (out[0] == fd || out[0] <= STDERR_FILENO) {
+ int newrfd;
+
+ newrfd = dup(out[0]);
+ if (newrfd >= 0 && newrfd <= STDERR_FILENO) {
+ /* this should not happen but try anyway */
+ (void)dup(newrfd);
+ }
If these dup() errors are ignored, this function may end up closing
the read end of the pipe in the subsequent dup2() call.
--
ldv