From e10b22f8e79ea73ff7afc7f35465012d1812d214 Mon Sep 17 00:00:00 2001 From: NIIBE Yutaka Date: Thu, 4 Jul 2024 10:55:21 +0900 Subject: [PATCH] Fix gnupg_create_{inbound,outbound}_pipe API. So that non-useful value for other end of pipe won't be shown to its users. Signed-off-by: NIIBE Yutaka --- common/exechelp-posix.c | 45 +++++++++++++---------- common/exechelp-w32.c | 81 +++++++++++++++++++++++------------------ common/exechelp.h | 12 +++--- common/exectool.c | 12 +++--- kbx/kbx-client-util.c | 18 ++++----- 5 files changed, 93 insertions(+), 75 deletions(-) diff --git a/common/exechelp-posix.c b/common/exechelp-posix.c index b78439dca..433c8249d 100644 --- a/common/exechelp-posix.c +++ b/common/exechelp-posix.c @@ -283,24 +283,31 @@ do_create_pipe (int filedes[2]) static gpg_error_t -create_pipe_and_estream (int filedes[2], estream_t *r_fp, +create_pipe_and_estream (int *r_fd, estream_t *r_fp, int outbound, int nonblock) { gpg_error_t err; + int filedes[2]; if (pipe (filedes) == -1) { err = my_error_from_syserror (); log_error (_("error creating a pipe: %s\n"), gpg_strerror (err)); - filedes[0] = filedes[1] = -1; + *r_fd = -1; *r_fp = NULL; return err; } if (!outbound) - *r_fp = es_fdopen (filedes[0], nonblock? "r,nonblock" : "r"); + { + *r_fd = filedes[1]; + *r_fp = es_fdopen (filedes[0], nonblock? "r,nonblock" : "r"); + } else - *r_fp = es_fdopen (filedes[1], nonblock? "w,nonblock" : "w"); + { + *r_fd = filedes[0]; + *r_fp = es_fdopen (filedes[1], nonblock? "w,nonblock" : "w"); + } if (!*r_fp) { err = my_error_from_syserror (); @@ -308,7 +315,7 @@ create_pipe_and_estream (int filedes[2], estream_t *r_fp, gpg_strerror (err)); close (filedes[0]); close (filedes[1]); - filedes[0] = filedes[1] = -1; + *r_fd = -1; return err; } return 0; @@ -316,28 +323,28 @@ create_pipe_and_estream (int filedes[2], estream_t *r_fp, /* Portable function to create a pipe. Under Windows the write end is - inheritable. If R_FP is not NULL, an estream is created for the - read end and stored at R_FP. */ + inheritable. Pipe is created and the read end is stored at R_FD. + An estream is created for the write end and stored at R_FP. */ gpg_error_t -gnupg_create_inbound_pipe (int filedes[2], estream_t *r_fp, int nonblock) +gnupg_create_inbound_pipe (int *r_fd, estream_t *r_fp, int nonblock) { - if (r_fp) - return create_pipe_and_estream (filedes, r_fp, 0, nonblock); - else - return do_create_pipe (filedes); + if (!r_fd || !r_fp) + gpg_error (GPG_ERR_INV_ARG); + + return create_pipe_and_estream (r_fd, r_fp, 0, nonblock); } /* Portable function to create a pipe. Under Windows the read end is - inheritable. If R_FP is not NULL, an estream is created for the - write end and stored at R_FP. */ + inheritable. Pipe is created and the write end is stored at R_FD. + An estream is created for the write end and stored at R_FP. */ gpg_error_t -gnupg_create_outbound_pipe (int filedes[2], estream_t *r_fp, int nonblock) +gnupg_create_outbound_pipe (int *r_fd, estream_t *r_fp, int nonblock) { - if (r_fp) - return create_pipe_and_estream (filedes, r_fp, 1, nonblock); - else - return do_create_pipe (filedes); + if (!r_fd || !r_fp) + gpg_error (GPG_ERR_INV_ARG); + + return create_pipe_and_estream (r_fd, r_fp, 1, nonblock); } diff --git a/common/exechelp-w32.c b/common/exechelp-w32.c index 6b1d9fa46..0c8314296 100644 --- a/common/exechelp-w32.c +++ b/common/exechelp-w32.c @@ -223,61 +223,72 @@ create_inheritable_pipe (HANDLE filedes[2], int flags) static gpg_error_t -create_pipe_and_estream (gnupg_fd_t fds[2], int flags, +create_pipe_and_estream (gnupg_fd_t *r_fd, int flags, estream_t *r_fp, int outbound, int nonblock) { gpg_error_t err = 0; es_syshd_t syshd; + gnupg_fd_t fds[2]; - fds[0] = fds[1] = GNUPG_INVALID_FD; if (create_inheritable_pipe (fds, flags) < 0) - err = my_error_from_syserror (); - - if (! err && r_fp) { - syshd.type = ES_SYSHD_HANDLE; - if (!outbound) - { - syshd.u.handle = fds[0]; - *r_fp = es_sysopen (&syshd, nonblock? "r,nonblock" : "r"); - } - else - { - syshd.u.handle = fds[1]; - *r_fp = es_sysopen (&syshd, nonblock? "w,nonblock" : "w"); - } - if (!*r_fp) - { - err = my_error_from_syserror (); - log_error (_("error creating a stream for a pipe: %s\n"), - gpg_strerror (err)); - CloseHandle (fds[0]); - CloseHandle (fds[1]); - fds[0] = fds[1] = GNUPG_INVALID_FD; - return err; - } + err = my_error_from_syserror (); + log_error (_("error creating a pipe: %s\n"), gpg_strerror (err)); + *r_fd = GNUPG_INVALID_FD; + *r_fp = NULL; + return err; } - return err; + syshd.type = ES_SYSHD_HANDLE; + if (!outbound) + { + syshd.u.handle = fds[0]; + *r_fd = fds[1]; + *r_fp = es_sysopen (&syshd, nonblock? "r,nonblock" : "r"); + } + else + { + syshd.u.handle = fds[1]; + *r_fd = fds[0]; + *r_fp = es_sysopen (&syshd, nonblock? "w,nonblock" : "w"); + } + if (!*r_fp) + { + err = my_error_from_syserror (); + log_error (_("error creating a stream for a pipe: %s\n"), + gpg_strerror (err)); + CloseHandle (fds[0]); + CloseHandle (fds[1]); + *r_fd = GNUPG_INVALID_FD; + return err; + } + + return 0; } /* Portable function to create a pipe. Under Windows the write end is - inheritable. If R_FP is not NULL, an estream is created for the - read end and stored at R_FP. */ + inheritable. Pipe is created and the read end is stored at R_FD. + An estream is created for the write end and stored at R_FP. */ gpg_error_t -gnupg_create_inbound_pipe (gnupg_fd_t fds[2], estream_t *r_fp, int nonblock) +gnupg_create_inbound_pipe (gnupg_fd_t *r_fd, estream_t *r_fp, int nonblock) { - return create_pipe_and_estream (fds, INHERIT_WRITE, r_fp, 0, nonblock); + if (!r_fd || !r_fp) + gpg_error (GPG_ERR_INV_ARG); + + return create_pipe_and_estream (r_fd, INHERIT_WRITE, r_fp, 0, nonblock); } /* Portable function to create a pipe. Under Windows the read end is - inheritable. If R_FP is not NULL, an estream is created for the - write end and stored at R_FP. */ + inheritable. Pipe is created and the write end is stored at R_FD. + An estream is created for the write end and stored at R_FP. */ gpg_error_t -gnupg_create_outbound_pipe (gnupg_fd_t fds[2], estream_t *r_fp, int nonblock) +gnupg_create_outbound_pipe (gnupg_fd_t *r_fd, estream_t *r_fp, int nonblock) { - return create_pipe_and_estream (fds, INHERIT_READ, r_fp, 1, nonblock); + if (!r_fd || !r_fp) + gpg_error (GPG_ERR_INV_ARG); + + return create_pipe_and_estream (r_fd, INHERIT_READ, r_fp, 1, nonblock); } diff --git a/common/exechelp.h b/common/exechelp.h index 9f9b70514..d2aac4b39 100644 --- a/common/exechelp.h +++ b/common/exechelp.h @@ -54,15 +54,15 @@ int *get_all_open_fds (void); /* Portable function to create a pipe. Under Windows the write end is - inheritable. If R_FP is not NULL, an estream is created for the - write end and stored at R_FP. */ -gpg_error_t gnupg_create_inbound_pipe (gnupg_fd_t filedes[2], + inheritable. Pipe is created and the read end is stored at R_FD. + An estream is created for the write end and stored at R_FP. */ +gpg_error_t gnupg_create_inbound_pipe (gnupg_fd_t *r_fd, estream_t *r_fp, int nonblock); /* Portable function to create a pipe. Under Windows the read end is - inheritable. If R_FP is not NULL, an estream is created for the - write end and stored at R_FP. */ -gpg_error_t gnupg_create_outbound_pipe (gnupg_fd_t filedes[2], + inheritable. Pipe is created and the write end is stored at R_FD. + An estream is created for the write end and stored at R_FP. */ +gpg_error_t gnupg_create_outbound_pipe (gnupg_fd_t *r_fd, estream_t *r_fp, int nonblock); /* Portable function to create a pipe. Under Windows both ends are diff --git a/common/exectool.c b/common/exectool.c index 2e54b79e1..de460084f 100644 --- a/common/exectool.c +++ b/common/exectool.c @@ -331,7 +331,7 @@ gnupg_exec_tool_stream (const char *pgmname, const char *argv[], #else int exceptclose[2]; #endif - gnupg_fd_t extrapipe[2] = { GNUPG_INVALID_FD, GNUPG_INVALID_FD }; + gnupg_fd_t extrapipe; char extrafdbuf[20]; const char *argsave = NULL; int argsaveidx; @@ -387,7 +387,7 @@ gnupg_exec_tool_stream (const char *pgmname, const char *argv[], if (inextra) { - err = gnupg_create_outbound_pipe (extrapipe, &extrafp, 1); + err = gnupg_create_outbound_pipe (&extrapipe, &extrafp, 1); if (err) { log_error ("error creating outbound pipe for extra fp: %s\n", @@ -395,7 +395,7 @@ gnupg_exec_tool_stream (const char *pgmname, const char *argv[], goto leave; } /* Do not close in child. */ - exceptclose[i] = extrapipe[0]; + exceptclose[i] = extrapipe; /* Now find the argument marker and replace by the pipe's fd. Yeah, that is an ugly non-thread safe hack but it safes us to create a copy of the array. */ @@ -439,11 +439,11 @@ gnupg_exec_tool_stream (const char *pgmname, const char *argv[], | GPGRT_PROCESS_STDERR_PIPE), act, &proc); gpgrt_process_get_streams (proc, GPGRT_PROCESS_STREAM_NONBLOCK, input? &infp : NULL, &outfp, &errfp); - if (extrapipe[0] != GNUPG_INVALID_FD) + if (extrapipe != GNUPG_INVALID_FD) #ifdef HAVE_W32_SYSTEM - CloseHandle (extrapipe[0]); + CloseHandle (extrapipe); #else - close (extrapipe[0]); + close (extrapipe); #endif if (argsave) argv[argsaveidx] = argsave; diff --git a/kbx/kbx-client-util.c b/kbx/kbx-client-util.c index 3a0489c6d..e7b5a6b7d 100644 --- a/kbx/kbx-client-util.c +++ b/kbx/kbx-client-util.c @@ -103,7 +103,7 @@ prepare_data_pipe (kbx_client_data_t kcd) { gpg_error_t err; int rc; - gnupg_fd_t inpipe[2]; + gnupg_fd_t inpipe; estream_t infp; npth_attr_t tattr; @@ -112,28 +112,28 @@ prepare_data_pipe (kbx_client_data_t kcd) kcd->datalen = 0; kcd->dataerr = 0; - err = gnupg_create_inbound_pipe (inpipe, &infp, 0); + err = gnupg_create_inbound_pipe (&inpipe, &infp, 0); if (err) { log_error ("error creating inbound pipe: %s\n", gpg_strerror (err)); return err; /* That should not happen. */ } - err = assuan_sendfd (kcd->ctx, inpipe[1]); + err = assuan_sendfd (kcd->ctx, inpipe); if (err) { #ifdef HAVE_W32_SYSTEM log_error ("sending fd %p to keyboxd: %s <%s>\n", - inpipe[1], gpg_strerror (err), gpg_strsource (err)); + inpipe, gpg_strerror (err), gpg_strsource (err)); #else log_error ("sending fd %d to keyboxd: %s <%s>\n", - inpipe[1], gpg_strerror (err), gpg_strsource (err)); + inpipe, gpg_strerror (err), gpg_strsource (err)); #endif es_fclose (infp); #ifdef HAVE_W32_SYSTEM - CloseHandle (inpipe[1]); + CloseHandle (inpipe); #else - close (inpipe[1]); + close (inpipe); #endif return err; } @@ -149,9 +149,9 @@ prepare_data_pipe (kbx_client_data_t kcd) } #ifdef HAVE_W32_SYSTEM - CloseHandle (inpipe[1]); + CloseHandle (inpipe); #else - close (inpipe[1]); + close (inpipe); #endif kcd->fp = infp;