Patch to allow pg_filedump to support reading of pg_filenode.map
Hello Hackers,
I recently had to work on a case where some catalog files were
corrupt and/or missing. One of the things we sought to inspect was
pg_filenode.map, but there was no tooling available to do so.
With the help of Álvaro H.. I've put together a patch to allow pg_filedump
to do some rudimentary decoding of pg_filenode.map, so that it's at least
human-readable. I had the idea to try to map the OIDs to relnames, but
that might get hairy, with TOAST table OIDs possibly changing (for pg_proc,
etc.)
It seems that Christoph Berg is the primary committer for the pg_filedump
project these days so he is cc'd here, but if there's some other place I
should be sending this kind of contribution to, please let me know.
Hopefully this will be helpful to others in the future.
Much appreciated,
--Richard
Attachments:
add_filenode_support.patchapplication/octet-stream; name=add_filenode_support.patchDownload+82-11
This is separate from the postgresql server repo.
https://git.postgresql.org/gitweb/?p=pg_filedump.git
+#define RELMAPPER_FILEMAGIC 0x592717
+char magic_buffer[8];
...
+ if ( (int) magic_buffer & RELMAPPER_FILEMAGIC ) {
This is doing bitwise arithmetic on a pointer, which seems badly wrong.
I think it breaks normal use of pg_filedump - unless you happen to get a
magic_buffer without those bits set. The segfault seems to confirm that, as
does gcc:
pg_filedump.c:2041:8: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
2041 | if ( (int) magic_buffer & RELMAPPER_FILEMAGIC ) {
I think it probably means to do memcmp, instead ??
--
Justin
Thanks for the feedback, Justin. I've gone ahead and switched to use
memcmp. I also refactored it to:
1. Don't assume that any file with first 4 bytes matching the
relmapper magic number is a pg_relnode.map file
2. Don't assume the pg_relnode.map file is uncorrupted and intact; perform
a check of the first 4 bytes against the reference magic number
3. Provide a flag (-m) for users to have their file interpreted as a
pg_relnode.map file
I hope this is more palatable to everyone :)
--Richard
On Wed, Apr 28, 2021 at 9:42 PM Justin Pryzby <pryzby@telsasoft.com> wrote:
Show quoted text
This is separate from the postgresql server repo.
https://git.postgresql.org/gitweb/?p=pg_filedump.git+#define RELMAPPER_FILEMAGIC 0x592717
+char magic_buffer[8];...
+ if ( (int) magic_buffer & RELMAPPER_FILEMAGIC ) {
This is doing bitwise arithmetic on a pointer, which seems badly wrong.
I think it breaks normal use of pg_filedump - unless you happen to get a
magic_buffer without those bits set. The segfault seems to confirm that,
as
does gcc:pg_filedump.c:2041:8: warning: cast from pointer to integer of different
size [-Wpointer-to-int-cast]
2041 | if ( (int) magic_buffer & RELMAPPER_FILEMAGIC ) {I think it probably means to do memcmp, instead ??
--
Justin
Attachments:
add_filenode_support_v2.patchapplication/octet-stream; name=add_filenode_support_v2.patchDownload+94-0
I think you should be able to avoid crashing if passed a non-relmapper file.
Make sure not to loop over more mappings than exist in the relmapper file of
the given size.
I guess you should warn if the number of mappings is too large for the file's
size. And then "cap" the number of mappings to the maximum possible number.
--
Justin
On Thu, Apr 29, 2021 at 12:05 PM Justin Pryzby <pryzby@telsasoft.com> wrote:
I think you should be able to avoid crashing if passed a non-relmapper
file.
Make sure not to loop over more mappings than exist in the relmapper file
of
the given size.I guess you should warn if the number of mappings is too large for the
file's
size. And then "cap" the number of mappings to the maximum possible
number.
Ah, thanks for the tip. That's right -- I can't assume the user's input is
a valid file. Updated patch here.
--Richard
Show quoted text
--
Justin
Attachments:
add_filenode_support_v3.patchapplication/octet-stream; name=add_filenode_support_v3.patchDownload+106-0
Re: Richard Yen
Ah, thanks for the tip. That's right -- I can't assume the user's input is
a valid file. Updated patch here.
Hi Richard,
sorry for the very late response here.
Thanks for the patch which I just merged, and thanks Justin for the
reviews!
Christoph
On Sep 29, 2021, at 9:01 AM, Christoph Berg <myon@debian.org> wrote:
Re: Richard Yen
Ah, thanks for the tip. That's right -- I can't assume the user's input is
a valid file. Updated patch here.Hi Richard,
sorry for the very late response here.
Thanks for the patch which I just merged, and thanks Justin for the
reviews!
Thank you! Was a great coding exercise :)
-Richard