gzgetc() is hazardous to your health

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: gzgetc() is hazardous to your health
Date: 2025-10-19 04:06:23
Message-ID: 2122679.1760846783@sss.pgh.pa.us
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

In the gift-that-keeps-on-giving department: I noticed a few days ago
while fooling with pg_dump's compression logic that our regression
tests were not reaching any of the getc_func methods. It turns out
that that's only used when reading a manually-compressed TOC file in
a directory-format dump, and we were not exercising that case. That
oversight already allowed one undetected bug (cf a239c4a0c), so I put
in 20ec99589 ... and mamba promptly fell over [1]. After a couple of
hours of digging I found that there's a configuration-sensitive bug in
NetBSD's 32-bit build of zlib [2]: if HAVE_UNISTD_H is defined before
including <zlib.h> then the gzgetc() macro ends up thinking that
gzFile_s.pos is 64 bits, while the library thinks it's 32. Ooops.

This can't be blamed entirely on NetBSD: zconf.h is hoary old code
that tries to deal with a lot of not-very-consistent platforms.
I think it likely that other platforms have similar issues, which
we'd not notice given that (a) we haven't been testing this case and
(b) probably few users are invoking it either and (c) it probably
only fails on 32-bit platforms.

What I think we ought to do about this is get rid of our one usage
of gzgetc(), replacing it with a one-byte gzread() operation.
That's not lovely from a speed perspective, but I don't think that
reading a pg_dump TOC file is really speed-critical.

regards, tom lane

PS: I've temporarily disabled --enable-tap-tests on mamba so
that it will go back to green. That's hardly a fix though.

[1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=mamba&dt=2025-10-17%2006%3A09%3A19
[2] https://gnats.netbsd.org/cgi-bin/query-pr-single.pl?number=59711

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message sunil s 2025-10-19 06:03:33 BRIN: Prevent the heapblk overflow during index summarization on very large tables resulting in an infinite loop
Previous Message Tatsuo Ishii 2025-10-19 01:02:58 Re: Add RESPECT/IGNORE NULLS and FROM FIRST/LAST options