Fix poor errno handling in libpq's version of our custom OpenSSL BIO.

tglsfdc · tglsfdc · commit b67c9c1939f1 · 2015-09-28T18:02:44.000-04:00
Thom Brown reported that SSL connections didn't seem to work on Windows in 9.5. Asif Naeem figured out that the cause was my_sock_read() looking at "errno" when it needs to look at "SOCK_ERRNO". This mistake was introduced in commit 680513a, which cloned the backend's custom SSL BIO code into libpq, and didn't translate the errno handling properly. Moreover, it introduced unnecessary errno save/restore logic, which was particularly confusing because it was incomplete; and it failed to check for all three of EINTR, EAGAIN, and EWOULDBLOCK in my_sock_write. (That might not be necessary; but since we're copying well-tested backend code that does do that, it seems prudent to copy it faithfully.)
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
@@ -1598,14 +1598,13 @@ static int
 my_sock_read(BIO *h, char *buf, int size)
 {
 	int			res;
-	int			save_errno;
 
 	res = pqsecure_raw_read((PGconn *) h->ptr, buf, size);
-	save_errno = errno;
 	BIO_clear_retry_flags(h);
 	if (res < 0)
 	{
-		switch (save_errno)
+		/* If we were interrupted, tell caller to retry */
+		switch (SOCK_ERRNO)
 		{
 #ifdef EAGAIN
 			case EAGAIN:
@@ -1622,24 +1621,33 @@ my_sock_read(BIO *h, char *buf, int size)
 		}
 	}
 
-	errno = save_errno;
 	return res;
 }
 
 static int
 my_sock_write(BIO *h, const char *buf, int size)
 {
 	int			res;
-	int			save_errno;
 
 	res = pqsecure_raw_write((PGconn *) h->ptr, buf, size);
-	save_errno = errno;
 	BIO_clear_retry_flags(h);
 	if (res <= 0)
 	{
-		if (save_errno == EINTR)
+		/* If we were interrupted, tell caller to retry */
+		switch (SOCK_ERRNO)
 		{
-			BIO_set_retry_write(h);
+#ifdef EAGAIN
+			case EAGAIN:
+#endif
+#if defined(EWOULDBLOCK) && (!defined(EAGAIN) || (EWOULDBLOCK != EAGAIN))
+			case EWOULDBLOCK:
+#endif
+			case EINTR:
+				BIO_set_retry_write(h);
+				break;
+
+			default:
+				break;
 		}
 	}
 

Original file line number	Diff line number	Diff line change
`@@ -1598,14 +1598,13 @@ static int`
`1598`	`1598`	`my_sock_read(BIO h, char buf, int size)`
`1599`	`1599`	`{`
`1600`	`1600`	`int res;`
`1601`		`- int save_errno;`
`1602`	`1601`
`1603`	`1602`	`res = pqsecure_raw_read((PGconn *) h->ptr, buf, size);`
`1604`		`- save_errno = errno;`
`1605`	`1603`	`BIO_clear_retry_flags(h);`
`1606`	`1604`	`if (res < 0)`
`1607`	`1605`	`{`
`1608`		`- switch (save_errno)`
	`1606`	`+ /* If we were interrupted, tell caller to retry */`
	`1607`	`+ switch (SOCK_ERRNO)`
`1609`	`1608`	`{`
`1610`	`1609`	`#ifdef EAGAIN`
`1611`	`1610`	`case EAGAIN:`
`@@ -1622,24 +1621,33 @@ my_sock_read(BIO h, char buf, int size)`
`1622`	`1621`	`}`
`1623`	`1622`	`}`
`1624`	`1623`
`1625`		`- errno = save_errno;`
`1626`	`1624`	`return res;`
`1627`	`1625`	`}`
`1628`	`1626`
`1629`	`1627`	`static int`
`1630`	`1628`	`my_sock_write(BIO h, const char buf, int size)`
`1631`	`1629`	`{`
`1632`	`1630`	`int res;`
`1633`		`- int save_errno;`
`1634`	`1631`
`1635`	`1632`	`res = pqsecure_raw_write((PGconn *) h->ptr, buf, size);`
`1636`		`- save_errno = errno;`
`1637`	`1633`	`BIO_clear_retry_flags(h);`
`1638`	`1634`	`if (res <= 0)`
`1639`	`1635`	`{`
`1640`		`- if (save_errno == EINTR)`
	`1636`	`+ /* If we were interrupted, tell caller to retry */`
	`1637`	`+ switch (SOCK_ERRNO)`
`1641`	`1638`	`{`
`1642`		`- BIO_set_retry_write(h);`
	`1639`	`+#ifdef EAGAIN`
	`1640`	`+ case EAGAIN:`
	`1641`	`+#endif`
	`1642`	`+#if defined(EWOULDBLOCK) && (!defined(EAGAIN) \|\| (EWOULDBLOCK != EAGAIN))`
	`1643`	`+ case EWOULDBLOCK:`
	`1644`	`+#endif`
	`1645`	`+ case EINTR:`
	`1646`	`+ BIO_set_retry_write(h);`
	`1647`	`+ break;`
	`1648`	`+`
	`1649`	`+ default:`
	`1650`	`+ break;`
`1643`	`1651`	`}`
`1644`	`1652`	`}`
`1645`	`1653`