From f716c3250adeed22f1ea1b1cb93e4782a202856a Mon Sep 17 00:00:00 2001 From: Bruce Momjian Date: Wed, 16 Apr 2014 10:45:48 -0400 Subject: [PATCH] check socket creation errors against PGINVALID_SOCKET Previously, in some places, socket creation errors were checked for negative values, which is not true for Windows because sockets are unsigned. This masked socket creation errors on Windows. Backpatch through 9.0. 8.4 doesn't have the infrastructure to fix this. --- src/backend/libpq/auth.c | 6 ++--- src/backend/libpq/ip.c | 10 ++++----- src/backend/libpq/pqcomm.c | 4 ++-- src/backend/port/win32/socket.c | 2 +- src/backend/postmaster/postmaster.c | 2 +- src/interfaces/libpq/fe-connect.c | 34 ++++++++++++++++++++++++----- src/interfaces/libpq/libpq-int.h | 1 + 7 files changed, 42 insertions(+), 17 deletions(-) diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c index 5e13145eff4..f3f3b71894b 100644 --- a/src/backend/libpq/auth.c +++ b/src/backend/libpq/auth.c @@ -1676,7 +1676,7 @@ ident_inet(hbaPort *port) sock_fd = socket(ident_serv->ai_family, ident_serv->ai_socktype, ident_serv->ai_protocol); - if (sock_fd < 0) + if (sock_fd == PGINVALID_SOCKET) { ereport(LOG, (errcode_for_socket_access(), @@ -1756,7 +1756,7 @@ ident_inet(hbaPort *port) ident_response))); ident_inet_done: - if (sock_fd >= 0) + if (sock_fd != PGINVALID_SOCKET) closesocket(sock_fd); pg_freeaddrinfo_all(remote_addr.addr.ss_family, ident_serv); pg_freeaddrinfo_all(local_addr.addr.ss_family, la); @@ -2583,7 +2583,7 @@ CheckRADIUSAuth(Port *port) packet->length = htons(packet->length); sock = socket(serveraddrs[0].ai_family, SOCK_DGRAM, 0); - if (sock < 0) + if (sock == PGINVALID_SOCKET) { ereport(LOG, (errmsg("could not create RADIUS socket: %m"))); diff --git a/src/backend/libpq/ip.c b/src/backend/libpq/ip.c index 3b827450af4..eb249e9df56 100644 --- a/src/backend/libpq/ip.c +++ b/src/backend/libpq/ip.c @@ -547,7 +547,7 @@ pg_foreach_ifaddr(PgIfAddrCallback callback, void *cb_data) int error; sock = WSASocket(AF_INET, SOCK_DGRAM, 0, 0, 0, 0); - if (sock == SOCKET_ERROR) + if (sock == INVALID_SOCKET) return -1; while (n_ii < 1024) @@ -670,7 +670,7 @@ pg_foreach_ifaddr(PgIfAddrCallback callback, void *cb_data) total; sock = socket(AF_INET, SOCK_DGRAM, 0); - if (sock == -1) + if (sock == PGINVALID_SOCKET) return -1; while (n_buffer < 1024 * 100) @@ -711,7 +711,7 @@ pg_foreach_ifaddr(PgIfAddrCallback callback, void *cb_data) #ifdef HAVE_IPV6 /* We'll need an IPv6 socket too for the SIOCGLIFNETMASK ioctls */ sock6 = socket(AF_INET6, SOCK_DGRAM, 0); - if (sock6 == -1) + if (sock6 == PGINVALID_SOCKET) { free(buffer); close(sock); @@ -788,10 +788,10 @@ pg_foreach_ifaddr(PgIfAddrCallback callback, void *cb_data) char *ptr, *buffer = NULL; size_t n_buffer = 1024; - int sock; + pgsocket sock; sock = socket(AF_INET, SOCK_DGRAM, 0); - if (sock == -1) + if (sock == PGINVALID_SOCKET) return -1; while (n_buffer < 1024 * 100) diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c index 76aac975528..516c559d9f3 100644 --- a/src/backend/libpq/pqcomm.c +++ b/src/backend/libpq/pqcomm.c @@ -392,7 +392,7 @@ StreamServerPort(int family, char *hostName, unsigned short portNumber, break; } - if ((fd = socket(addr->ai_family, SOCK_STREAM, 0)) < 0) + if ((fd = socket(addr->ai_family, SOCK_STREAM, 0)) == PGINVALID_SOCKET) { ereport(LOG, (errcode_for_socket_access(), @@ -632,7 +632,7 @@ StreamConnection(pgsocket server_fd, Port *port) port->raddr.salen = sizeof(port->raddr.addr); if ((port->sock = accept(server_fd, (struct sockaddr *) & port->raddr.addr, - &port->raddr.salen)) < 0) + &port->raddr.salen)) == PGINVALID_SOCKET) { ereport(LOG, (errcode_for_socket_access(), diff --git a/src/backend/port/win32/socket.c b/src/backend/port/win32/socket.c index 0a132c4b82e..e349511fedf 100644 --- a/src/backend/port/win32/socket.c +++ b/src/backend/port/win32/socket.c @@ -132,7 +132,7 @@ int pgwin32_waitforsinglesocket(SOCKET s, int what, int timeout) { static HANDLE waitevent = INVALID_HANDLE_VALUE; - static SOCKET current_socket = -1; + static SOCKET current_socket = INVALID_SOCKET; static int isUDP = 0; HANDLE events[2]; int r; diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 99f98205dd7..6977e43a8f6 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -2146,7 +2146,7 @@ ConnCreate(int serverFd) if (StreamConnection(serverFd, port) != STATUS_OK) { - if (port->sock >= 0) + if (port->sock != PGINVALID_SOCKET) StreamClose(port->sock); ConnFree(port); return NULL; diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index b2f547a8d64..5f1bfb801eb 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -1631,8 +1631,23 @@ keep_going: /* We will come back to here until there is conn->raddr.salen = addr_cur->ai_addrlen; /* Open a socket */ - conn->sock = socket(addr_cur->ai_family, SOCK_STREAM, 0); - if (conn->sock < 0) + { + /* + * While we use 'pgsocket' as the socket type in the + * backend, we use 'int' for libpq socket values. + * This requires us to map PGINVALID_SOCKET to -1 + * on Windows. + * See http://msdn.microsoft.com/en-us/library/windows/desktop/ms740516%28v=vs.85%29.aspx + */ + pgsocket sock = socket(addr_cur->ai_family, SOCK_STREAM, 0); +#ifdef WIN32 + if (sock == PGINVALID_SOCKET) + conn->sock = -1; + else +#endif + conn->sock = sock; + } + if (conn->sock == -1) { /* * ignore socket() failure if we have more addresses @@ -3136,7 +3151,7 @@ internal_cancel(SockAddr *raddr, int be_pid, int be_key, char *errbuf, int errbufsize) { int save_errno = SOCK_ERRNO; - int tmpsock = -1; + pgsocket tmpsock = PGINVALID_SOCKET; char sebuf[256]; int maxlen; struct @@ -3149,7 +3164,7 @@ internal_cancel(SockAddr *raddr, int be_pid, int be_key, * We need to open a temporary connection to the postmaster. Do this with * only kernel calls. */ - if ((tmpsock = socket(raddr->addr.ss_family, SOCK_STREAM, 0)) < 0) + if ((tmpsock = socket(raddr->addr.ss_family, SOCK_STREAM, 0)) == PGINVALID_SOCKET) { strlcpy(errbuf, "PQcancel() -- socket() failed: ", errbufsize); goto cancel_errReturn; @@ -3220,7 +3235,7 @@ cancel_errReturn: maxlen); strcat(errbuf, "\n"); } - if (tmpsock >= 0) + if (tmpsock != PGINVALID_SOCKET) closesocket(tmpsock); SOCK_ERRNO_SET(save_errno); return FALSE; @@ -5281,6 +5296,15 @@ PQerrorMessage(const PGconn *conn) return conn->errorMessage.data; } +/* + * In Windows, socket values are unsigned, and an invalid socket value + * (INVALID_SOCKET) is ~0, which equals -1 in comparisons (with no compiler + * warning). Ideally we would return an unsigned value for PQsocket() on + * Windows, but that would cause the function's return value to differ from + * Unix, so we just return -1 for invalid sockets. + * http://msdn.microsoft.com/en-us/library/windows/desktop/cc507522%28v=vs.85%29.aspx + * http://stackoverflow.com/questions/10817252/why-is-invalid-socket-defined-as-0-in-winsock2-h-c + */ int PQsocket(const PGconn *conn) { diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index 408aeb136b6..11f66e1b4d7 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -364,6 +364,7 @@ struct pg_conn PGnotify *notifyTail; /* newest unreported Notify msg */ /* Connection data */ + /* See PQconnectPoll() for how we use 'int' and not 'pgsocket'. */ int sock; /* Unix FD for socket, -1 if not connected */ SockAddr laddr; /* Local address */ SockAddr raddr; /* Remote address */