diff options
author | Robert Haas <rhaas@postgresql.org> | 2023-03-27 13:27:06 -0400 |
---|---|---|
committer | Robert Haas <rhaas@postgresql.org> | 2023-03-27 13:36:11 -0400 |
commit | 80d5e3a615518e9eee8ba4afa5cf05ebe486dbf6 (patch) | |
tree | 9af2ee7464c1e7f5a39b316217f035796d8178c6 /contrib/amcheck | |
parent | 5a91c79755034fecb546cbee76c14b89c514177c (diff) | |
download | postgresql-80d5e3a615518e9eee8ba4afa5cf05ebe486dbf6.tar.gz |
amcheck: Tighten up validation of redirect line pointers.
Commit bbc1376b39627c6bddd8a0dc0a7dda24c91a97a0 added a new lp_valid[]
array which records whether or not a line pointer was thought to be
valid, but entries could sometimes get set to true in cases where that
wasn't actually safe. Fix that.
Suppose A is a redirect line pointer and B is the other line pointer
to which it points. The old code could mishandle this situation in a
couple of different ways. First, if B was unused, we'd complain about
corruption but still set lp_valid[A] = true, causing later code
to try to access the B as if it were pointing to a tuple. Second,
if B was dead, we wouldn't complain about corruption at all, which is
an oversight, and would also set lp_valid[A] = true, which would
again confuse later code. Fix all that.
In the case where B is a redirect, the old code was correct, but
refactor things a bit anyway so that all of these cases are handled
more symmetrically. Also add an Assert() and some comments.
Andres Freund and Robert Haas
Discussion: http://postgr.es/m/20230323172607.y3lejpntjnuis5vv%40awork3.anarazel.de
Diffstat (limited to 'contrib/amcheck')
-rw-r--r-- | contrib/amcheck/verify_heapam.c | 40 |
1 files changed, 31 insertions, 9 deletions
diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c index 10dd31477b..1dc9d89f30 100644 --- a/contrib/amcheck/verify_heapam.c +++ b/contrib/amcheck/verify_heapam.c @@ -481,11 +481,35 @@ verify_heapam(PG_FUNCTION_ARGS) (unsigned) maxoff)); continue; } + + /* + * Since we've checked that this redirect points to a line + * pointer between FirstOffsetNumber and maxoff, it should + * now be safe to fetch the referenced line pointer. We expect + * it to be LP_NORMAL; if not, that's corruption. + */ rditem = PageGetItemId(ctx.page, rdoffnum); if (!ItemIdIsUsed(rditem)) + { report_corruption(&ctx, - psprintf("line pointer redirection to unused item at offset %u", + psprintf("redirected line pointer points to an unused item at offset %u", (unsigned) rdoffnum)); + continue; + } + else if (ItemIdIsDead(rditem)) + { + report_corruption(&ctx, + psprintf("redirected line pointer points to a dead item at offset %u", + (unsigned) rdoffnum)); + continue; + } + else if (ItemIdIsRedirected(rditem)) + { + report_corruption(&ctx, + psprintf("redirected line pointer points to another redirected line pointer at offset %u", + (unsigned) rdoffnum)); + continue; + } /* * Record the fact that this line pointer has passed basic @@ -584,14 +608,12 @@ verify_heapam(PG_FUNCTION_ARGS) /* Handle the cases where the current line pointer is a redirect. */ if (ItemIdIsRedirected(curr_lp)) { - /* Can't redirect to another redirect. */ - if (ItemIdIsRedirected(next_lp)) - { - report_corruption(&ctx, - psprintf("redirected line pointer points to another redirected line pointer at offset %u", - (unsigned) nextoffnum)); - continue; - } + /* + * We should not have set successor[ctx.offnum] to a value + * other than InvalidOffsetNumber unless that line pointer + * is LP_NORMAL. + */ + Assert(ItemIdIsNormal(next_lp)); /* Can only redirect to a HOT tuple. */ next_htup = (HeapTupleHeader) PageGetItem(ctx.page, next_lp); |