Re: Assert in pageinspect with NULL pages

On Fri, Mar 25, 2022 at 12:57 AM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> On Thu, Mar 24, 2022 at 08:54:14PM -0700, Peter Geoghegan wrote:
> > amcheck's palloc_btree_page() function validates that an 8KiB page is
> > in fact an nbtree page, in a maximally paranoid way. Might be an
> > example worth following here.
>
> Sure, we could do that. However, I don't think that going down to
> that is something we have any need for in pageinspect, as the module
> is also useful to look at the contents of the full page, even if
> slightly corrupted, and too many checks would prevent a lookup at the
> internal contents of a page.

It's my impression that there are many ways of crashing the system
using pageinspect right now. We aren't very careful about making sure
that our code that reads from disk doesn't crash if the data on disk
is corrupted, and all of that systemic weakness is inherited by
pageinspect. But, with pageinspect, you can supply your own pages, not
just use the ones that actually exist on disk.

Fixing this seems like a lot of work and problematic for various
reasons. And I do agree that if we can allow people to look at what's
going on with a corrupt page, that's useful. On the other hand, if by
"looking" they crash the system, that sucks a lot. So overall I think
we need a lot more sanity checks here.

--
Robert Haas
EDB: http://www.enterprisedb.com