public inbox for [email protected]  
help / color / mirror / Atom feed
From: Daniil Davydov <[email protected]>
To: [email protected]
Subject: Improve consistency checks in tbm_prepare_shared_iterate
Date: Wed, 25 Dec 2024 19:44:03 +0700
Message-ID: <CAJDiXgh4CX7u95TvJH7LXShtuJ3Fu0-gYdLHfGwt8RZ5k27Hmg@mail.gmail.com> (raw)

Hi,
Currently, iterating through TIDBitmap contains this code (REL_16_STABLE) :
***
while ((page = pagetable_iterate(tbm->pagetable, &i)) != NULL)
{
    idx = page - ptbase->ptentry;
    if (page->ischunk)
        ptchunks->index[nchunks++] = idx;
    else
        ptpages->index[npages++] = idx;
}

Assert(npages == tbm->npages);
Assert(nchunks == tbm->nchunks);
***

Two asserts in the end seem to be overdue to me, because if (for
example) nchunks > tbm->nchunks (due to another error),
we have already accessed to invalid memory chunk and overwritten it.
If we want to monitor the correctness of these variables, it might be
better to add a few checks, as in the attached patch.

I'm not sure if the comment in the error message is written correctly,
but first I would like to hear your opinion.

--
Best regards,
Daniil Davydov


Attachments:

  [text/x-patch] 0001-Add-new-checks-for-tidbitmap-scan.patch (1.2K, ../CAJDiXgh4CX7u95TvJH7LXShtuJ3Fu0-gYdLHfGwt8RZ5k27Hmg@mail.gmail.com/2-0001-Add-new-checks-for-tidbitmap-scan.patch)
  download | inline diff:
From 04e41969ef9ea2c96af218a5519b5efb19baeed5 Mon Sep 17 00:00:00 2001
From: Daniil Davidov <[email protected]>
Date: Wed, 25 Dec 2024 19:32:55 +0700
Subject: [PATCH] Add new checks for tidbitmap scan

---
 src/backend/nodes/tidbitmap.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c
index 29a18584410..34a015e5ea7 100644
--- a/src/backend/nodes/tidbitmap.c
+++ b/src/backend/nodes/tidbitmap.c
@@ -827,10 +827,17 @@ tbm_prepare_shared_iterate(TIDBitmap *tbm)
 			while ((page = pagetable_iterate(tbm->pagetable, &i)) != NULL)
 			{
 				idx = page - ptbase->ptentry;
-				if (page->ischunk)
+				if (page->ischunk && (nchunks + 1 <= tbm->nchunks))
 					ptchunks->index[nchunks++] = idx;
-				else
+				else if (npages + 1 <= tbm->npages)
 					ptpages->index[npages++] = idx;
+				else
+					ereport(ERROR,
+							(errcode(ERRCODE_INTERNAL_ERROR),
+							 errmsg("invalid chunks/pages num in tidbitmap : %d"
+							 		" chunks met of %d available, %d pages met "
+									"of %d available", nchunks, tbm->nchunks,
+									npages, tbm->npages)));
 			}
 
 			Assert(npages == tbm->npages);
-- 
2.43.0



view thread (2+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected]
  Subject: Re: Improve consistency checks in tbm_prepare_shared_iterate
  In-Reply-To: <CAJDiXgh4CX7u95TvJH7LXShtuJ3Fu0-gYdLHfGwt8RZ5k27Hmg@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox