Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle SIGPIPE in s2nd/s2nc #69

Closed
colmmacc opened this issue Apr 7, 2015 · 0 comments · Fixed by #70
Closed

Handle SIGPIPE in s2nd/s2nc #69

colmmacc opened this issue Apr 7, 2015 · 0 comments · Fixed by #70

Comments

@colmmacc
Copy link
Contributor

colmmacc commented Apr 7, 2015

Mikko from codenomicon reported occasionally seeing SIGPIPE as an unhandled signal in s2nd. SIGPIPE can be generated when we try to write() to the remote end and the remote end has closed. I don't think that libs2n should mask this; our design is to emulate POSIX read()/write(), but we need s2nd and s2nc to be more tolerant of this. Instead of receiving the signal, it would be better to handle the error in the normal ways, with write() or read() returning negative values (or EOF for read()).

Following patch suppresses SIGPIPE:

index b0268c1..0562d53 100644
--- a/bin/s2nd.c
+++ b/bin/s2nd.c
@@ -22,6 +22,7 @@
 #include <stdlib.h>
 #include <unistd.h>
 #include <string.h>
+#include <signal.h>
 #include <stdio.h>

 #include <errno.h>
@@ -117,6 +118,11 @@ int main(int argc, const char *argv[])
     hints.ai_family = AF_UNSPEC;
     hints.ai_socktype = SOCK_STREAM;

+    if (signal(SIGPIPE, SIG_IGN) == SIG_ERR) {
+        fprintf(stderr, "Error disabling SIGPIPE\n");
+        exit(1);
+    }
+
     if ((r = getaddrinfo(argv[1], argv[2], &hints, &ai)) < 0) {
         fprintf(stderr, "getaddrinfo error: %s\n", gai_strerror(r));
         return -1;
colmmacc added a commit that referenced this issue Jun 29, 2015
This change modifies s2nd and s2nc to ignore SIGPIPE rather than
have it as an unhandled exception (which exits the process).
This problem was reported by Mikko at Codenomicon. Fixes #69

Additionally this change also modifies our "return -1" cases
to exit(1), since these occur in main().
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant