aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--changes/bug62066
-rw-r--r--src/common/compat.c54
-rw-r--r--src/common/compat.h2
-rw-r--r--src/ext/eventdns.c8
-rw-r--r--src/or/connection.c23
-rw-r--r--src/or/cpuworker.c7
6 files changed, 82 insertions, 18 deletions
diff --git a/changes/bug6206 b/changes/bug6206
new file mode 100644
index 000000000..61a16d291
--- /dev/null
+++ b/changes/bug6206
@@ -0,0 +1,6 @@
+ o Minor bugfixes:
+ - Always check the return values of functions fcntl() and
+ setsockopt(). We don't believe these are ever actually failing in
+ practice, but better safe than sorry. Also, checking these return
+ values should please some analysis tools (like Coverity). Patch
+ from 'flupzor'. Fix for bug 8206; bugfix on all versions of Tor.
diff --git a/src/common/compat.c b/src/common/compat.c
index d7ce89479..25df9a960 100644
--- a/src/common/compat.c
+++ b/src/common/compat.c
@@ -137,8 +137,13 @@ tor_open_cloexec(const char *path, int flags, unsigned mode)
fd = open(path, flags, mode);
#ifdef FD_CLOEXEC
- if (fd >= 0)
- fcntl(fd, F_SETFD, FD_CLOEXEC);
+ if (fd >= 0) {
+ if (fcntl(fd, F_SETFD, FD_CLOEXEC) == -1) {
+ log_warn(LD_FS,"Couldn't set FD_CLOEXEC: %s", strerror(errno));
+ close(fd);
+ return -1;
+ }
+ }
#endif
return fd;
}
@@ -150,8 +155,13 @@ tor_fopen_cloexec(const char *path, const char *mode)
{
FILE *result = fopen(path, mode);
#ifdef FD_CLOEXEC
- if (result != NULL)
- fcntl(fileno(result), F_SETFD, FD_CLOEXEC);
+ if (result != NULL) {
+ if (fcntl(fileno(result), F_SETFD, FD_CLOEXEC) == -1) {
+ log_warn(LD_FS,"Couldn't set FD_CLOEXEC: %s", strerror(errno));
+ fclose(result);
+ return NULL;
+ }
+ }
#endif
return result;
}
@@ -1024,7 +1034,15 @@ tor_open_socket(int domain, int type, int protocol)
return s;
#if defined(FD_CLOEXEC)
- fcntl(s, F_SETFD, FD_CLOEXEC);
+ if (fcntl(s, F_SETFD, FD_CLOEXEC) == -1) {
+ log_warn(LD_FS,"Couldn't set FD_CLOEXEC: %s", strerror(errno));
+#if defined(_WIN32)
+ closesocket(s);
+#else
+ close(s);
+#endif
+ return -1;
+ }
#endif
goto socket_ok; /* So that socket_ok will not be unused. */
@@ -1059,7 +1077,11 @@ tor_accept_socket(tor_socket_t sockfd, struct sockaddr *addr, socklen_t *len)
return s;
#if defined(FD_CLOEXEC)
- fcntl(s, F_SETFD, FD_CLOEXEC);
+ if (fcntl(s, F_SETFD, FD_CLOEXEC) == -1) {
+ log_warn(LD_NET, "Couldn't set FD_CLOEXEC: %s", strerror(errno));
+ close(s);
+ return TOR_INVALID_SOCKET;
+ }
#endif
goto socket_ok; /* So that socket_ok will not be unused. */
@@ -1083,17 +1105,31 @@ get_n_open_sockets(void)
return n;
}
-/** Turn <b>socket</b> into a nonblocking socket.
+/** Turn <b>socket</b> into a nonblocking socket. Return 0 on success, -1
+ * on failure.
*/
-void
+int
set_socket_nonblocking(tor_socket_t socket)
{
#if defined(_WIN32)
unsigned long nonblocking = 1;
ioctlsocket(socket, FIONBIO, (unsigned long*) &nonblocking);
#else
- fcntl(socket, F_SETFL, O_NONBLOCK);
+ int flags;
+
+ flags = fcntl(socket, F_GETFL, 0);
+ if (flags == -1) {
+ log_warn(LD_NET, "Couldn't get file status flags: %s", strerror(errno));
+ return -1;
+ }
+ flags |= O_NONBLOCK;
+ if (fcntl(socket, F_SETFL, flags) == -1) {
+ log_warn(LD_NET, "Couldn't set file status flags: %s", strerror(errno));
+ return -1;
+ }
#endif
+
+ return 0;
}
/**
diff --git a/src/common/compat.h b/src/common/compat.h
index f9eb4ba0b..f0a34aae4 100644
--- a/src/common/compat.h
+++ b/src/common/compat.h
@@ -518,7 +518,7 @@ int tor_inet_aton(const char *cp, struct in_addr *addr) ATTR_NONNULL((1,2));
const char *tor_inet_ntop(int af, const void *src, char *dst, size_t len);
int tor_inet_pton(int af, const char *src, void *dst);
int tor_lookup_hostname(const char *name, uint32_t *addr) ATTR_NONNULL((1,2));
-void set_socket_nonblocking(tor_socket_t socket);
+int set_socket_nonblocking(tor_socket_t socket);
int tor_socketpair(int family, int type, int protocol, tor_socket_t fd[2]);
int network_init(void);
diff --git a/src/ext/eventdns.c b/src/ext/eventdns.c
index 3ee9f7245..0dd7629a1 100644
--- a/src/ext/eventdns.c
+++ b/src/ext/eventdns.c
@@ -2275,6 +2275,7 @@ _evdns_nameserver_add_impl(const struct sockaddr *address,
const struct nameserver *server = server_head, *const started_at = server_head;
struct nameserver *ns;
+ int flags;
int err = 0;
if (server) {
@@ -2306,7 +2307,12 @@ _evdns_nameserver_add_impl(const struct sockaddr *address,
ioctlsocket(ns->socket, FIONBIO, &nonblocking);
}
#else
- fcntl(ns->socket, F_SETFL, O_NONBLOCK);
+ if (fcntl(ns->socket, F_SETFL, O_NONBLOCK) == -1) {
+ evdns_log(EVDNS_LOG_WARN, "Error %s (%d) while settings file status flags.",
+ tor_socket_strerror(errno), errno);
+ err = 2;
+ goto out2;
+ }
#endif
if (global_bind_addr_is_set &&
diff --git a/src/or/connection.c b/src/or/connection.c
index c659e65fe..622eadcff 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -918,8 +918,11 @@ make_socket_reuseable(tor_socket_t sock)
* right after somebody else has let it go. But REUSEADDR on win32
* means you can bind to the port _even when somebody else
* already has it bound_. So, don't do that on Win32. */
- setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, (void*) &one,
- (socklen_t)sizeof(one));
+ if (setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, (void*) &one,
+ (socklen_t)sizeof(one)) == -1) {
+ log_warn(LD_NET, "Error setting SO_REUSEADDR flag: %s",
+ tor_socket_strerror(errno));
+ }
#endif
}
@@ -1102,7 +1105,10 @@ connection_listener_new(const struct sockaddr *listensockaddr,
tor_assert(0);
}
- set_socket_nonblocking(s);
+ if (set_socket_nonblocking(s) == -1) {
+ tor_close_socket(s);
+ goto err;
+ }
lis_conn = listener_connection_new(type, listensockaddr->sa_family);
conn = TO_CONN(lis_conn);
@@ -1265,7 +1271,10 @@ connection_handle_listener_read(connection_t *conn, int new_type)
(int)news,(int)conn->s);
make_socket_reuseable(news);
- set_socket_nonblocking(news);
+ if (set_socket_nonblocking(news) == -1) {
+ tor_close_socket(news);
+ return 0;
+ }
if (options->ConstrainedSockets)
set_constrained_socket_buffers(news, (int)options->ConstrainedSockSize);
@@ -1494,7 +1503,11 @@ connection_connect(connection_t *conn, const char *address,
}
}
- set_socket_nonblocking(s);
+ if (set_socket_nonblocking(s) == -1) {
+ *socket_error = tor_socket_errno(s);
+ tor_close_socket(s);
+ return -1;
+ }
if (options->ConstrainedSockets)
set_constrained_socket_buffers(s, (int)options->ConstrainedSockSize);
diff --git a/src/or/cpuworker.c b/src/or/cpuworker.c
index 38c6613f0..61f9faa39 100644
--- a/src/or/cpuworker.c
+++ b/src/or/cpuworker.c
@@ -535,13 +535,16 @@ spawn_cpuworker(void)
conn = connection_new(CONN_TYPE_CPUWORKER, AF_UNIX);
- set_socket_nonblocking(fd);
-
/* set up conn so it's got all the data we need to remember */
conn->s = fd;
conn->address = tor_strdup("localhost");
tor_addr_make_unspec(&conn->addr);
+ if (set_socket_nonblocking(fd) == -1) {
+ connection_free(conn); /* this closes fd */
+ return -1;
+ }
+
if (connection_add(conn) < 0) { /* no space, forget it */
log_warn(LD_NET,"connection_add for cpuworker failed. Giving up.");
connection_free(conn); /* this closes fd */
generated by cgit v1.2.3 (git 2.25.4) at 2024-05-23 18:05:59 +0000