| From: | Daria Lepikhova <daria(dot)n(dot)lepikhova(at)gmail(dot)com> |
|---|---|
| To: | pgsql-hackers(at)postgresql(dot)org |
| Subject: | pg_verifybackup / pg_combinebackup: missing readdir() error check in directory scan |
| Date: | 2026-09-21 16:57:26 |
| Message-ID: | CAK49S1CmBUMJNT3CNBLDf1Xaz=_J70Gs_-t6LJM=vEMub2oL_g@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
While reading pg_verifybackup sources I noticed that the directory scan
never
checks errno after the readdir() loop, although similar loops elsewhere in
the tree does. And pg_combinebackup has the same problem:
while (errno = 0, (dirent = readdir(dir)) != NULL)
{
...
}
A failure partway through the scan is then indistinguishable from the end
of the directory. pg_verifybackup reports the entries it never reached as
"present in the manifest but not on disk"; pg_combinebackup writes out the
truncated directory and exits normally.
There are four such loops: verify_plain_backup_directory() and
verify_tar_backup() in pg_verifybackup.c, process_directory_recursively()
and scan_for_existing_tablespaces() in pg_combinebackup.c.
Commit 6f03927fce (Properly check for readdir/closedir() failures) added
the errno check to pg_archivecleanup.c, fd.c, initdb.c and other call
sites. pg_verifybackup and pg_combinebackup came later, which is probably
why the check never made it into their code.
The attached patch makes small changes to pg_verifybackup and
pg_combinebackup, adding the missing checks. pg_fatal() seems sufficient
for pg_combinebackup. For pg_verifybackup I don't think
report_backup_error() alone is enough: it keeps going, so the bogus
"missing file" reports remain and merely gain one more line. The patch
adds the directory to the ignore list as well, which is what the same
function already does when opendir() fails.
I simulated a readdir() failure on a plain backup of 21000 files.
pg_verifybackup reported 16732 of them as missing, though everyone was on
disk. With the patch it reports a single "could not read directory" error.
Unfortunately I couldn't come up with a portable test that would
demonstrate this; if someone can, I'd be grateful. The pattern seems clear
enough even without one.
--
Daria Lepikhova
| Attachment | Content-Type | Size |
|---|---|---|
| v1-0001-Check-for-readdir-failures-in-pg_verifybackup-and.patch | application/octet-stream | 2.5 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Hannu Krosing | 2026-09-21 18:29:14 | Re: Direct TOAST v2, faster, smaller and no migration needed |
| Previous Message | Alvaro Herrera | 2026-09-21 16:56:27 | Re: Race conditions in logical decoding |