Skip to content

Commit 1fb4b08

Browse files
jchampioCommitfest Bot
authored andcommitted
libpq: Extend "read pending" check from SSL to GSS
An extra check for pending bytes in the SSL layer has been part of pqReadReady() for a very long time (79ff2e9). But when GSS transport encryption was added, it didn't receive the same treatment. (As 79ff2e9 notes, "The bug that I fixed in this patch is exceptionally hard to reproduce reliably.") Without that check, it's possible to hit a hang in gssencmode, if the server splits a large libpq message such that the final message in a streamed response is part of the same wrapped token as the split message: DataRowDataRowDataRowDataRowDataRowData -- token boundary -- RowDataRowCommandCompleteReadyForQuery If the split message takes up enough memory to nearly fill libpq's receive buffer, libpq may return from pqReadData() before the later messages are pulled out of the PqGSSRecvBuffer. Without additional socket activity from the server, pqReadReady() (via pqSocketCheck()) will never again return true, hanging the connection. Pull the pending-bytes check into the pqsecure API layer, where both SSL and GSS now implement it. Note that this does not fix the root problem! Third party clients of libpq have no way to call pqsecure_read_is_pending() in their own polling. This just brings the GSS implementation up to par with the existing SSL workaround; a broader fix is left to a subsequent commit. (However, pgtls_read_pending() is renamed to pgtls_read_is_pending(), to avoid conflation with the forthcoming pgtls_drain_pending().) Discussion: https://postgr.es/m/CAOYmi%2BmpymrgZ76Jre2dx_PwRniS9YZojwH0rZnTuiGHCsj0rA%40mail.gmail.com
1 parent 5310fac commit 1fb4b08

File tree

5 files changed

+31
-6
lines changed

5 files changed

+31
-6
lines changed

src/interfaces/libpq/fe-misc.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1099,14 +1099,12 @@ pqSocketCheck(PGconn *conn, int forRead, int forWrite, pg_usec_time_t end_time)
10991099
return -1;
11001100
}
11011101

1102-
#ifdef USE_SSL
1103-
/* Check for SSL library buffering read bytes */
1104-
if (forRead && conn->ssl_in_use && pgtls_read_pending(conn))
1102+
/* Check for SSL/GSS library buffering read bytes */
1103+
if (forRead && pqsecure_read_is_pending(conn))
11051104
{
11061105
/* short-circuit the select */
11071106
return 1;
11081107
}
1109-
#endif
11101108
}
11111109

11121110
/* We will retry as long as we get EINTR */

src/interfaces/libpq/fe-secure-gssapi.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -471,6 +471,12 @@ gss_read(PGconn *conn, void *recv_buffer, size_t length, ssize_t *ret)
471471
return PGRES_POLLING_OK;
472472
}
473473

474+
bool
475+
pg_GSS_read_is_pending(PGconn *conn)
476+
{
477+
return PqGSSResultLength > PqGSSResultNext;
478+
}
479+
474480
/*
475481
* Negotiate GSSAPI transport for a connection. When complete, returns
476482
* PGRES_POLLING_OK. Will return PGRES_POLLING_READING or

src/interfaces/libpq/fe-secure-openssl.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ pgtls_read(PGconn *conn, void *ptr, size_t len)
231231
}
232232

233233
bool
234-
pgtls_read_pending(PGconn *conn)
234+
pgtls_read_is_pending(PGconn *conn)
235235
{
236236
return SSL_pending(conn->ssl) > 0;
237237
}

src/interfaces/libpq/fe-secure.c

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,25 @@ pqsecure_raw_read(PGconn *conn, void *ptr, size_t len)
243243
return n;
244244
}
245245

246+
/*
247+
* Returns true if there are any bytes available in the transport buffer.
248+
*/
249+
bool
250+
pqsecure_read_is_pending(PGconn *conn)
251+
{
252+
#ifdef USE_SSL
253+
if (conn->ssl_in_use)
254+
return pgtls_read_is_pending(conn);
255+
#endif
256+
#ifdef ENABLE_GSS
257+
if (conn->gssenc)
258+
return pg_GSS_read_is_pending(conn);
259+
#endif
260+
261+
/* Plaintext connections have no transport buffer. */
262+
return 0;
263+
}
264+
246265
/*
247266
* Write data to a secure connection.
248267
*

src/interfaces/libpq/libpq-int.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -810,6 +810,7 @@ extern int pqWriteReady(PGconn *conn);
810810
extern PostgresPollingStatusType pqsecure_open_client(PGconn *);
811811
extern void pqsecure_close(PGconn *);
812812
extern ssize_t pqsecure_read(PGconn *, void *ptr, size_t len);
813+
extern bool pqsecure_read_is_pending(PGconn *);
813814
extern ssize_t pqsecure_write(PGconn *, const void *ptr, size_t len);
814815
extern ssize_t pqsecure_raw_read(PGconn *, void *ptr, size_t len);
815816
extern ssize_t pqsecure_raw_write(PGconn *, const void *ptr, size_t len);
@@ -848,7 +849,7 @@ extern ssize_t pgtls_read(PGconn *conn, void *ptr, size_t len);
848849
/*
849850
* Is there unread data waiting in the SSL read buffer?
850851
*/
851-
extern bool pgtls_read_pending(PGconn *conn);
852+
extern bool pgtls_read_is_pending(PGconn *conn);
852853

853854
/*
854855
* Write data to a secure connection.
@@ -896,6 +897,7 @@ extern PostgresPollingStatusType pqsecure_open_gss(PGconn *conn);
896897
*/
897898
extern ssize_t pg_GSS_write(PGconn *conn, const void *ptr, size_t len);
898899
extern ssize_t pg_GSS_read(PGconn *conn, void *ptr, size_t len);
900+
extern bool pg_GSS_read_is_pending(PGconn *conn);
899901
#endif
900902

901903
/* === in fe-trace.c === */

0 commit comments

Comments
 (0)