From fc21e154b9e0db833e5a156e87ba106b47c4cd6c Mon Sep 17 00:00:00 2001 From: Gerhard Rieger Date: Mon, 6 Nov 2023 21:42:11 +0100 Subject: [PATCH] Check pipe size for possible blocking --- CHANGES | 4 ++++ socat.c | 24 ++++++++++++------------ xio-pipe.c | 41 +++++++++++++++++++++++++++++++++++++++++ xio-pipe.h | 2 ++ xio-progcall.c | 2 +- xio.h | 3 ++- xioparam.c | 3 ++- 7 files changed, 64 insertions(+), 15 deletions(-) diff --git a/CHANGES b/CHANGES index f1b79b6..8e5fb7a 100644 --- a/CHANGES +++ b/CHANGES @@ -166,6 +166,10 @@ Features: Filan prints the current value. Tests: STDIN_F_SETPIPE_SZ EXEC_F_SETPIPE_SZ + Bidirectional PIPE addresses may block on writing a data chunk larger + than pipe buffer. Socat now tries to detect if transfer block size is + large enough and issues a warning. + Corrections: When a sub process (EXEC, SYSTEM) terminated with exit code other than 0, its last sent data might have been lost depending on timing of read/ diff --git a/socat.c b/socat.c index 8632093..c857b0b 100644 --- a/socat.c +++ b/socat.c @@ -22,10 +22,11 @@ #include "xioopts.h" #include "xiolockfile.h" +#include "xio-pipe.h" + /* command line options */ -struct { - size_t bufsiz; +struct socat_opts { bool verbose; bool verbhex; struct timeval pollintv; /* with ignoreeof, reread after seconds */ @@ -40,7 +41,6 @@ struct { unsigned long log_sigs; /* signals to be caught just for logging */ bool statistics; /* log statistics on exit */ } socat_opts = { - 8192, /* bufsiz */ false, /* verbose */ false, /* verbhex */ {1,0}, /* pollintv */ @@ -234,7 +234,7 @@ int main(int argc, const char *argv[]) { Exit(1); } } - socat_opts.bufsiz = Strtoul(a, (char **)&a, 0, "-b"); + xioparms.bufsiz = Strtoul(a, (char **)&a, 0, "-b"); break; case 's': if (arg1[0][2]) { socat_opt_hint(stderr, arg1[0][1], arg1[0][2]); Exit(1); } diag_set_int('e', E_FATAL); break; @@ -936,9 +936,9 @@ int _socat(void) { #endif /* WITH_FILAN */ /* when converting nl to crnl, size might double */ - if (socat_opts.bufsiz > (SIZE_MAX-1)/2) { - Error2("buffer size option (-b) to big - "F_Zu" (max is "F_Zu")", socat_opts.bufsiz, (SIZE_MAX-1)/2); - socat_opts.bufsiz = (SIZE_MAX-1)/2; + if (xioparms.bufsiz > (SIZE_MAX-1)/2) { + Error2("buffer size option (-b) to big - "F_Zu" (max is "F_Zu")", xioparms.bufsiz, (SIZE_MAX-1)/2); + xioparms.bufsiz = (SIZE_MAX-1)/2; } #if HAVE_PROTOTYPE_LIB_posix_memalign @@ -946,13 +946,13 @@ int _socat(void) { Without this, eg.read() fails with "Invalid argument" */ { int _errno; - if ((_errno = Posix_memalign((void **)&buff, getpagesize(), 2*socat_opts.bufsiz+1)) != 0) { + if ((_errno = Posix_memalign((void **)&buff, getpagesize(), 2*xioparms.bufsiz+1)) != 0) { Error1("posix_memalign(): %s", strerror(_errno)); return -1; } } #else /* !HAVE_PROTOTYPE_LIB_posix_memalign */ - buff = Malloc(2*socat_opts.bufsiz+1); + buff = Malloc(2*xioparms.bufsiz+1); if (buff == NULL) return -1; #endif /* !HAVE_PROTOTYPE_LIB_posix_memalign */ @@ -1180,7 +1180,7 @@ int _socat(void) { if (mayrd1 && maywr2) { mayrd1 = false; - if ((bytes1 = xiotransfer(sock1, sock2, buff, socat_opts.bufsiz, false)) + if ((bytes1 = xiotransfer(sock1, sock2, buff, xioparms.bufsiz, false)) < 0) { if (errno != EAGAIN) { closing = MAX(closing, 1); @@ -1212,7 +1212,7 @@ int _socat(void) { if (mayrd2 && maywr1) { mayrd2 = false; - if ((bytes2 = xiotransfer(sock2, sock1, buff, socat_opts.bufsiz, true)) + if ((bytes2 = xiotransfer(sock2, sock1, buff, xioparms.bufsiz, true)) < 0) { if (errno != EAGAIN) { closing = MAX(closing, 1); @@ -1633,7 +1633,7 @@ int cv_newline(unsigned char *buff, ssize_t *bytes, from = '\r'; } if (buf2 == NULL) { - if ((buf2 = Malloc(socat_opts.bufsiz)) == NULL) { + if ((buf2 = Malloc(xioparms.bufsiz)) == NULL) { return -1; } } diff --git a/xio-pipe.c b/xio-pipe.c index 718333f..da17b8a 100644 --- a/xio-pipe.c +++ b/xio-pipe.c @@ -74,6 +74,9 @@ static int xioopen_fifo_unnamed(xiofile_t *sock, struct opt *opts) { showleft(opts); Error1("INTERNAL: %d option(s) remained unused", numleft); } + + xio_chk_pipesz(sfd->fd); + Notice("writing to and reading from unnamed pipe"); return 0; } @@ -183,7 +186,45 @@ static int xioopen_fifo( applyopts_named(pipename, opts, PH_FD); applyopts(sfd, -1, opts, PH_FD); applyopts_cloexec(sfd->fd, opts); + xio_chk_pipesz(sfd->fd); + return _xio_openlate(sfd, opts); } + +/* Checks if fd is a pipe and if its buffer is at least the blksiz. + returns 0 if ok; + returns 1 if unknown; + returns -1 if not */ +int xio_chk_pipesz( + int fd) +{ + struct stat st; + int pipesz; + + if (fstat(fd, &st) < 0) { + Warn2("fstat(%d, ...): %s", fd, strerror(errno)); + return 1; + } + if ((st.st_mode&S_IFMT) != S_IFIFO) { + return 0; + } + +#if defined(F_GETPIPE_SZ) + if ((pipesz = Fcntl(fd, F_GETPIPE_SZ)) < 0) { + Warn2("fcntl(%d, F_GETPIPE_SZ): %s", fd, strerror(errno)); + return 1; + } + + if (pipesz >= xioparms.bufsiz) + return 0; + + Warn3("xio_chk_pipesz(%d, ...): Socat block size "F_Zu" is larger than pipe size %d, might block; use option f-setpipe-sz!", + fd, xioparms.bufsiz, pipesz); + return -1; +#else /* !defined(F_GETPIPE_SZ) */ + return 1; +#endif /* !defined(F_GETPIPE_SZ) */ +} + #endif /* WITH_PIPE */ diff --git a/xio-pipe.h b/xio-pipe.h index 7171e6b..ef3e06b 100644 --- a/xio-pipe.h +++ b/xio-pipe.h @@ -9,4 +9,6 @@ extern const struct addrdesc xioaddr_pipe; extern const struct optdesc opt_f_setpipe_sz; +extern int xio_chk_pipesz(int fd); + #endif /* !defined(__xio_pipe_h_included) */ diff --git a/xio-progcall.c b/xio-progcall.c index 5a0f54d..9b302f0 100644 --- a/xio-progcall.c +++ b/xio-progcall.c @@ -330,7 +330,7 @@ int _xioopen_foxec(int xioflags, /* XIO_RDONLY etc. */ if (rw != XIO_RDONLY) { applyopts_cloexec(wrpip[1], popts); if (sfd->dtype == XIODATA_2PIPE) - applyopts(NULL, wrpip[1], popts, PH_FD); + applyopts(NULL, wrpip[1], popts2, PH_FD); else applyopts(NULL, wrpip[1], popts, PH_FD); applyopts(NULL, wrpip[0], copts, PH_FD); diff --git a/xio.h b/xio.h index 9af9d96..71bddc6 100644 --- a/xio.h +++ b/xio.h @@ -104,7 +104,7 @@ typedef uint32_t groups_t; #endif /* global XIO options/parameters */ -typedef struct { +typedef struct xioparms { bool strictopts; const char *pipesep; const char *paramsep; @@ -119,6 +119,7 @@ typedef struct { bool experimental; /* enable some features */ const char *sniffleft_name; /* file name with -r */ const char *sniffright_name; /* file name with -R */ + size_t bufsiz; } xioparms_t; /* pack the description of a lock file */ diff --git a/xioparam.c b/xioparam.c index 11830c5..b2b1406 100644 --- a/xioparam.c +++ b/xioparam.c @@ -23,7 +23,8 @@ xioparms_t xioparms = { WITH_DEFAULT_IPV, /* preferred_ip */ false, /* experimental */ NULL, /* sniffleft_name */ - NULL /* sniffright_name */ + NULL, /* sniffright_name */ + 8192 /* bufsiz */ } ;