Re: [PATCH 1/3] Check output of write() to stdout

[Ted Unangst]

On Mon, Jun 02, 2014 at 09:51, Brent Cook wrote:

> Is something like this more to taste? Or maybe just a simple (void)write()
> as in libc/time/zic.c ?

I think maybe we want to save this one for later. If it's not
immediately obvious what the correct fix is, move along to something
where the correct fix is obvious.

Re: [PATCH 1/3] Check output of write() to stdout

[Brent Cook]

> abort? are you insane? no no no no... 


Me IRL: http://cdn.memegenerator.net/instances/400x/37703326.jpg

That’s the problem with swiss army knife programs; too many corner cases. I was 
thinking about why SIGPIPE exists and something like this:

cat /dev/urandom | openssl s_server -quiet | crashy_filter

Is something like this more to taste? Or maybe just a simple (void)write() as 
in libc/time/zic.c ?

diff --git a/src/apps/s_server.c b/src/apps/s_server.c
index 77384ec..8c866e0 100644
--- a/src/apps/s_server.c
+++ b/src/apps/s_server.c
@@ -1497,6 +1497,24 @@ print_stats(BIO * bio, SSL_CTX * ssl_ctx)
SSL_CTX_sess_get_cache_size(ssl_ctx));
 }

+static int write_stdout_buf(char *buf, int i)
+{
+   int written, n;
+   for (written = 0; written < i;) {
+   do {
+   n = write(fileno(stdout), buf + written, i - written);
+   } while (n == -1 && errno == EINTR);
+
+   if (n > 0) {
+   written += n;
+   } else if (errno != EAGAIN && errno != EWOULDBLOCK) {
+   BIO_printf(bio_err, "Could note write to stdout: %s\n", 
strerror(errno));
+   return -1;
+   }
+   }
+   return written;
+}
+
 static int
 sv_body(char *hostname, int s, unsigned char *context)
 {
@@ -1760,8 +1778,7 @@ sv_body(char *hostname, int s, unsigned char *context)
i = SSL_read(con, (char *) buf, bufsize);
switch (SSL_get_error(con, i)) {
case SSL_ERROR_NONE:
-   write(fileno(stdout), buf,
-   (unsigned int) i);
+   write_stdout_buf(buf, i);
if (SSL_pending(con))
goto again;
break;


On Jun 2, 2014, at 7:58 AM, Bob Beck  wrote:

> 
> 
> On Sun, Jun 1, 2014 at 8:28 PM, Brent Cook  wrote:
> Check for errors on write. Since SIGPIPE is ignored, play nicely with
> pipelines by aborting on EPIPE.
> ---
>  src/apps/s_server.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/src/apps/s_server.c b/src/apps/s_server.c
> index 77384ec..836d46b 100644
> --- a/src/apps/s_server.c
> +++ b/src/apps/s_server.c
> @@ -1760,8 +1760,11 @@ sv_body(char *hostname, int s, unsigned char *context)
> i = SSL_read(con, (char *) buf, bufsize);
> switch (SSL_get_error(con, i)) {
> case SSL_ERROR_NONE:
> -   write(fileno(stdout), buf,
> -   (unsigned int) i);
> +   if (write(fileno(stdout), buf, i) == 
> -1) {
> +   if (errno == EPIPE) {
> +   abort();
> +   }
> +   }
> if (SSL_pending(con))
> goto again;
> break;
> --
> 1.9.3
> 
> 

Re: [PATCH 1/3] Check output of write() to stdout

[Bob Beck]

abort? are you insane? no no no no...


On Sun, Jun 1, 2014 at 8:28 PM, Brent Cook  wrote:

> Check for errors on write. Since SIGPIPE is ignored, play nicely with
> pipelines by aborting on EPIPE.
> ---
>  src/apps/s_server.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/src/apps/s_server.c b/src/apps/s_server.c
> index 77384ec..836d46b 100644
> --- a/src/apps/s_server.c
> +++ b/src/apps/s_server.c
> @@ -1760,8 +1760,11 @@ sv_body(char *hostname, int s, unsigned char
> *context)
> i = SSL_read(con, (char *) buf, bufsize);
> switch (SSL_get_error(con, i)) {
> case SSL_ERROR_NONE:
> -   write(fileno(stdout), buf,
> -   (unsigned int) i);
> +   if (write(fileno(stdout), buf, i)
> == -1) {
> +   if (errno == EPIPE) {
> +   abort();
> +   }
> +   }
> if (SSL_pending(con))
> goto again;
> break;
> --
> 1.9.3
>
>

Re: [PATCH 1/3] Check output of write() to stdout

[Theo de Raadt]

This diff is very dissapointing.


> Check for errors on write. Since SIGPIPE is ignored, play nicely with
> pipelines by aborting on EPIPE.
> ---
>  src/apps/s_server.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/src/apps/s_server.c b/src/apps/s_server.c
> index 77384ec..836d46b 100644
> --- a/src/apps/s_server.c
> +++ b/src/apps/s_server.c
> @@ -1760,8 +1760,11 @@ sv_body(char *hostname, int s, unsigned char *context)
>   i = SSL_read(con, (char *) buf, bufsize);
>   switch (SSL_get_error(con, i)) {
>   case SSL_ERROR_NONE:
> - write(fileno(stdout), buf,
> - (unsigned int) i);
> + if (write(fileno(stdout), buf, i) == 
> -1) {
> + if (errno == EPIPE) {
> + abort();
> + }
> + }
>   if (SSL_pending(con))
>   goto again;
>   break;
> -- 
> 1.9.3
> 

[PATCH 1/3] Check output of write() to stdout

[Brent Cook]

Check for errors on write. Since SIGPIPE is ignored, play nicely with
pipelines by aborting on EPIPE.
---
 src/apps/s_server.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/apps/s_server.c b/src/apps/s_server.c
index 77384ec..836d46b 100644
--- a/src/apps/s_server.c
+++ b/src/apps/s_server.c
@@ -1760,8 +1760,11 @@ sv_body(char *hostname, int s, unsigned char *context)
i = SSL_read(con, (char *) buf, bufsize);
switch (SSL_get_error(con, i)) {
case SSL_ERROR_NONE:
-   write(fileno(stdout), buf,
-   (unsigned int) i);
+   if (write(fileno(stdout), buf, i) == 
-1) {
+   if (errno == EPIPE) {
+   abort();
+   }
+   }
if (SSL_pending(con))
goto again;
break;
-- 
1.9.3