Обсуждение: 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
Вложения
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:
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
Вложения
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
--
Justin
Вложения
> 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