| From: | Tatsuo Ishii <ishii(at)postgresql(dot)org> |
|---|---|
| To: | pgpool-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Stricter check for frontend message kind |
| Date: | 2026-08-26 23:44:02 |
| Message-ID: | 20260827.084402.441949153144326686.ishii@postgresql.org |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgpool-hackers |
While looking into pgpool source code for some reasons, I realize that
we can enhance pgpool to implement stricter check for frontend message
kinds. Please note that this is not a proposal, just a starting point
of discussion. I feel there maybe a better idea for this.
In ProcessFrontendResponse() we process messages from frontend in
following order (I focus on the v3 protocol path here):
1. read message kind
2. read message content length
3. convert the result of 2 to host byte order and subtract 4 byte from
it to get the actual message content length.
4. read message content using ppool_read2
If the message from frontend is broken or out of sync, we could read
garbage 4 bytes from frontend and treat it as message content
length. If the message kind is 'Q' and the length is too short, the
message is forwarded to PostgreSQL and PostgreSQL rejects the message
with an error response. Since at this point the protocol is out of
sync, subsequent message could generate random error including a hang
in pgpool.
I temporarily modified pgproto to allow to generate such a broken
message content length, by adding new 'q' message kind (which is not
defined in frontend/backend protocol). Following is the pgproto data.
'q' is similar to 'Q' (query message) but accepts message content
lenngth + 4 in the next field to 'q'. The message is sent as 'Q'
message.
Following 'q' message supposes to send query string (9 bytes including
NUL terminate) + 4 = 13 bytes message content length, but deliberately
set it to 10.
--------------------------------------------------------------------
'q' 10 "SELECT 1"
'Y'
--------------------------------------------------------------------
If I run this, I get this response.
<= BE ErrorResponse(S ERROR V ERROR C 08P01 M invalid string in message F pqformat.c L 593 R pq_getmsgstring )
This means pgpool forwarded the message to PostgreSQL and it raised an
error.
If the pgproto data includes following normal SELECT after 'q':
'Q' "SELECT 1"
'Y'
pgpool is stuck in pool_read2, trying to read the message content
which could be interpreted like length = 822104304 (in the example
above).
If I send the same data to directly PostgreSQL, I get following
responses, resulted in client disconnection, instead of a hang.
<= BE ErrorResponse(S ERROR V ERROR C 08P01 M invalid string in message F pqformat.c L 593 R pq_getmsgstring )
<= BE ErrorResponse(S FATAL V FATAL C 08P01 M invalid frontend message type 32 F postgres.c L 454 R SocketBackend )
This is because PostgreSQL checks the message kind and rejects the
invalid message kind with a FATAL error. IMO, we should follow the
PostgreSQL way so that we could avoid the hang in pool_read2 like
this:
FE=> Query (query="SELECT 1")
<= BE ErrorResponse(S ERROR V ERROR C 08P01 M invalid string in message F pqformat.c L 593 R pq_getmsgstring )
<= BE ReadyForQuery(I)
FE=> Query (query="SELECT 1")
<= BE ErrorResponse(S FATAL C XX000 M invalid frontend message type ' ' F pool_proto_modules.c L 2913 )
read_it: read(2) returns error Connection reset by peer
Patch attached.
(Also pgproto patch attached).
Regards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp
| Attachment | Content-Type | Size |
|---|---|---|
| ProcessFrontendResponse.patch | text/x-patch | 874 bytes |
| pgproto.patch | text/x-patch | 874 bytes |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tatsuo Ishii | 2026-08-26 23:48:42 | Re: Stricter check for frontend message kind |
| Previous Message | Koshino Taiki | 2026-08-26 09:21:31 | Convert pcp_worker die handler to flag-only. |