Horton

Re: Assert in pageinspect with NULL pages

Michael Paquier <michael@paquier.xyz>

View thread View on postgr.es
From: Michael Paquier <michael@paquier.xyz>
To: Peter Geoghegan <pg@bowt.ie>
Cc: Robert Haas <robertmhaas@gmail.com>, Alexander Lakhin <exclusion@gmail.com>, Justin Pryzby <pryzby@telsasoft.com>, Daria Lepikhova <d.lepikhova@postgrespro.ru>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2022-03-28T03:39:55Z
Lists: pgsql-hackers 
On Sun, Mar 27, 2022 at 02:36:54PM -0700, Peter Geoghegan wrote:
> I think that it might actually be fundamentally impossible to
> guarantee that that'll be safe, because we have no idea what the
> output function might be doing. It's arbitrary user-defined code that
> could easily be implemented in C. Combined with an arbitrary page
> image.

My guess that you'd bring in a lot more safety if we completely cut
the capacity to fetch and pass down raw pages across the SQL calls
because you can add checks for the wanted AM, replacing all that with
a (regclass,blkno) pair.  I am however ready to buy that this may not
be worth rewriting 10~15 years after the fact.

> I think that most of the functions can approach being perfectly
> robust, with a little work. In practical terms they can almost
> certainly be made so robust that no real user of bt_page_items() will
> ever crash the server. Somebody that goes out of their way to do that
> *might* find a way (even with the easier cases), but that doesn't
> particularly concern me.

That does not concern me either, and my own take is to take as
realistic things that can be fetched from a get_raw_page() call rather
than a bytea specifically crafted.  Now, I have found myself in cases
where it was useful to see the contents of a page, as long as you can 
go through the initial checks, particularly in cases where corruptions
involved unaligned contents.  Trigerring an error on a sanity check
for a specific block may be fine, now I have also found myself doing
some full scans to see in one shot the extent of a damaged relation
file using the functions of pageinspect.  Fun times.
--
Michael

Commits

  1. pageinspect: Fix handling of all-zero pages

  2. pageinspect: Add more sanity checks to prevent out-of-bound reads

  3. pageinspect: Fix handling of page sizes and AM types

  4. Teach pageinspect about nbtree deduplication.