Horton

Re: Assert in pageinspect with NULL pages

Robert Haas <robertmhaas@gmail.com>

View thread View on postgr.es
From: Robert Haas <robertmhaas@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Peter Geoghegan <pg@bowt.ie>, 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-27T14:24:14Z
Lists: pgsql-hackers 
On Fri, Mar 25, 2022 at 12:57 AM Michael Paquier <michael@paquier.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

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.