]> granicus.if.org Git - postgresql/commitdiff
Add more sanity-checking to PageAddItem and PageIndexTupleDelete,
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 15 Jan 2002 22:14:17 +0000 (22:14 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 15 Jan 2002 22:14:17 +0000 (22:14 +0000)
to prevent spreading of corruption when page header pointers are bad.
Merge PageZero into PageInit, since it was never used separately, and
remove separate memset calls used at most other PageInit call points.
Remove IndexPageCleanup, which wasn't used at all.

src/backend/access/gist/gist.c
src/backend/access/hash/hashpage.c
src/backend/access/heap/heapam.c
src/backend/access/nbtree/nbtpage.c
src/backend/access/rtree/rtree.c
src/backend/storage/page/bufpage.c
src/include/storage/bufpage.h

index 38fe008e8ea13bf9da861cd5164851295121852b..db57a5f3ffaa3b4a87fa236745688deca20cca25 100644 (file)
@@ -8,7 +8,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/access/gist/gist.c,v 1.86 2001/11/05 17:46:23 momjian Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/access/gist/gist.c,v 1.87 2002/01/15 22:14:16 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1437,7 +1437,7 @@ GISTInitBuffer(Buffer b, uint32 f)
        pageSize = BufferGetPageSize(b);
 
        page = BufferGetPage(b);
-       MemSet(page, 0, (int) pageSize);
+
        PageInit(page, pageSize, sizeof(GISTPageOpaqueData));
 
        opaque = (GISTPageOpaque) PageGetSpecialPointer(page);
index 0fff5a11e6faf57b0e7c1edbba1284671f2603ff..668f0b16d097e1f95f338270d5689110007bb607 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/access/hash/hashpage.c,v 1.33 2001/10/25 05:49:21 momjian Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/access/hash/hashpage.c,v 1.34 2002/01/15 22:14:16 tgl Exp $
  *
  * NOTES
  *       Postgres hash pages look like ordinary relation pages.  The opaque
@@ -301,17 +301,7 @@ _hash_chgbufaccess(Relation rel,
 void
 _hash_pageinit(Page page, Size size)
 {
-       Assert(((PageHeader) page)->pd_lower == 0);
-       Assert(((PageHeader) page)->pd_upper == 0);
-       Assert(((PageHeader) page)->pd_special == 0);
-
-       /*
-        * Cargo-cult programming -- don't really need this to be zero, but
-        * creating new pages is an infrequent occurrence and it makes me feel
-        * good when I know they're empty.
-        */
-       MemSet(page, 0, size);
-
+       Assert(PageIsNew(page));
        PageInit(page, size, sizeof(HashPageOpaqueData));
 }
 
index 5073a2e1900070425bc3e12a6308b294958502fc..878896d5e271846eec0b6216a7fb9765e769cb3d 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/access/heap/heapam.c,v 1.128 2001/11/05 17:46:23 momjian Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/access/heap/heapam.c,v 1.129 2002/01/15 22:14:17 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -2094,10 +2094,7 @@ heap_xlog_insert(bool redo, XLogRecPtr lsn, XLogRecord *record)
                uint32          newlen;
 
                if (record->xl_info & XLOG_HEAP_INIT_PAGE)
-               {
                        PageInit(page, BufferGetPageSize(buffer), 0);
-                       PageZero(page);
-               }
 
                if (XLByteLE(lsn, PageGetLSN(page)))    /* changes are applied */
                {
@@ -2262,10 +2259,7 @@ newsame:;
                uint32          newlen;
 
                if (record->xl_info & XLOG_HEAP_INIT_PAGE)
-               {
                        PageInit(page, BufferGetPageSize(buffer), 0);
-                       PageZero(page);
-               }
 
                if (XLByteLE(lsn, PageGetLSN(page)))    /* changes are applied */
                {
index 2e6eb20cd4ce270df88b6e0cfbd86f65537959df..17006ed06ce534453ec0a5f6d99652acc4c0f04c 100644 (file)
@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/access/nbtree/nbtpage.c,v 1.54 2001/10/25 05:49:21 momjian Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/access/nbtree/nbtpage.c,v 1.55 2002/01/15 22:14:17 tgl Exp $
  *
  *     NOTES
  *        Postgres btree pages look like ordinary relation pages.      The opaque
@@ -399,14 +399,6 @@ _bt_wrtnorelbuf(Relation rel, Buffer buf)
 void
 _bt_pageinit(Page page, Size size)
 {
-       /*
-        * Cargo_cult programming -- don't really need this to be zero, but
-        * creating new pages is an infrequent occurrence and it makes me feel
-        * good when I know they're empty.
-        */
-
-       MemSet(page, 0, size);
-
        PageInit(page, size, sizeof(BTPageOpaqueData));
        ((BTPageOpaque) PageGetSpecialPointer(page))->btpo_parent =
                InvalidBlockNumber;
index 1e74678831f59d8355a1e9262b732cc9edce1149..2526ea4b812aedf4f918025cdda986d8514f51a8 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/access/rtree/Attic/rtree.c,v 1.68 2001/11/05 17:46:24 momjian Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/access/rtree/Attic/rtree.c,v 1.69 2002/01/15 22:14:17 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1107,7 +1107,7 @@ RTInitBuffer(Buffer b, uint32 f)
        pageSize = BufferGetPageSize(b);
 
        page = BufferGetPage(b);
-       MemSet(page, 0, (int) pageSize);
+
        PageInit(page, pageSize, sizeof(RTreePageOpaqueData));
 
        opaque = (RTreePageOpaque) PageGetSpecialPointer(page);
index 916071aa098eb463551cd47454633ae9050b7ec0..d6c0339e142b1b1296946601a4cf3486c695053d 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/storage/page/bufpage.c,v 1.41 2001/11/08 04:05:13 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/storage/page/bufpage.c,v 1.42 2002/01/15 22:14:17 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
 #include "storage/bufpage.h"
 
 
-static void PageIndexTupleDeleteAdjustLinePointers(PageHeader phdr,
-                                                                          char *location, Size size);
-
-
 /* ----------------------------------------------------------------
  *                                             Page support functions
  * ----------------------------------------------------------------
@@ -44,24 +40,15 @@ PageInit(Page page, Size pageSize, Size specialSize)
        Assert(pageSize >
                   specialSize + sizeof(PageHeaderData) - sizeof(ItemIdData));
 
+       /* Make sure all fields of page are zero, as well as unused space */
+       MemSet(p, 0, pageSize);
+
        p->pd_lower = sizeof(PageHeaderData) - sizeof(ItemIdData);
        p->pd_upper = pageSize - specialSize;
        p->pd_special = pageSize - specialSize;
        PageSetPageSize(page, pageSize);
-
-       p->pd_lsn.xlogid = p->pd_lsn.xrecoff = 0;
-       p->pd_sui = 0;
 }
 
-/*
- * WAL needs in zero-ed page data content
- */
-void
-PageZero(Page page)
-{
-       MemSet((char *) page + ((PageHeader) page)->pd_lower, 0,
-               ((PageHeader) page)->pd_special - ((PageHeader) page)->pd_lower);
-}
 
 /* ----------------
  *             PageAddItem
@@ -86,10 +73,11 @@ PageAddItem(Page page,
                        OffsetNumber offsetNumber,
                        ItemIdFlags flags)
 {
+       PageHeader      phdr = (PageHeader) page;
        int                     i;
        Size            alignedSize;
-       Offset          lower;
-       Offset          upper;
+       int                     lower;
+       int                     upper;
        ItemId          itemId;
        OffsetNumber limit;
        bool            needshuffle = false;
@@ -97,6 +85,16 @@ PageAddItem(Page page,
 
        flags &= ~OverwritePageMode;
 
+       /*
+        * Be wary about corrupted page pointers
+        */
+       if (phdr->pd_lower < (sizeof(PageHeaderData) - sizeof(ItemIdData)) ||
+               phdr->pd_lower > phdr->pd_upper ||
+               phdr->pd_upper > phdr->pd_special ||
+               phdr->pd_special > BLCKSZ)
+               elog(ERROR, "PageAddItem: corrupted page pointers: lower = %u, upper = %u, special = %u",
+                        phdr->pd_lower, phdr->pd_upper, phdr->pd_special);
+
        /*
         * Find first unallocated offsetNumber
         */
@@ -114,7 +112,7 @@ PageAddItem(Page page,
                        }
                        if (offsetNumber < limit)
                        {
-                               itemId = &((PageHeader) page)->pd_linp[offsetNumber - 1];
+                               itemId = &phdr->pd_linp[offsetNumber - 1];
                                if (((*itemId).lp_flags & LP_USED) ||
                                        ((*itemId).lp_len != 0))
                                {
@@ -138,7 +136,7 @@ PageAddItem(Page page,
                /* look for "recyclable" (unused & deallocated) ItemId */
                for (offsetNumber = 1; offsetNumber < limit; offsetNumber++)
                {
-                       itemId = &((PageHeader) page)->pd_linp[offsetNumber - 1];
+                       itemId = &phdr->pd_linp[offsetNumber - 1];
                        if ((((*itemId).lp_flags & LP_USED) == 0) &&
                                ((*itemId).lp_len == 0))
                                break;
@@ -146,18 +144,21 @@ PageAddItem(Page page,
        }
 
        /*
-        * Compute new lower and upper pointers for page, see if it'll fit
+        * Compute new lower and upper pointers for page, see if it'll fit.
+        *
+        * Note: do arithmetic as signed ints, to avoid mistakes if, say,
+        * alignedSize > pd_upper.
         */
        if (offsetNumber > limit)
-               lower = (Offset) (((char *) (&((PageHeader) page)->pd_linp[offsetNumber])) - ((char *) page));
+               lower = (char *) (&phdr->pd_linp[offsetNumber]) - (char *) page;
        else if (offsetNumber == limit || needshuffle)
-               lower = ((PageHeader) page)->pd_lower + sizeof(ItemIdData);
+               lower = phdr->pd_lower + sizeof(ItemIdData);
        else
-               lower = ((PageHeader) page)->pd_lower;
+               lower = phdr->pd_lower;
 
        alignedSize = MAXALIGN(size);
 
-       upper = ((PageHeader) page)->pd_upper - alignedSize;
+       upper = (int) phdr->pd_upper - (int) alignedSize;
 
        if (lower > upper)
                return InvalidOffsetNumber;
@@ -169,24 +170,26 @@ PageAddItem(Page page,
        if (needshuffle)
        {
                /* shuffle ItemId's (Do the PageManager Shuffle...) */
-               for (i = (limit - 1); i >= offsetNumber; i--)
+               for (i = (int) limit - 1; i >= (int) offsetNumber; i--)
                {
                        ItemId          fromitemId,
                                                toitemId;
 
-                       fromitemId = &((PageHeader) page)->pd_linp[i - 1];
-                       toitemId = &((PageHeader) page)->pd_linp[i];
+                       fromitemId = &phdr->pd_linp[i - 1];
+                       toitemId = &phdr->pd_linp[i];
                        *toitemId = *fromitemId;
                }
        }
 
-       itemId = &((PageHeader) page)->pd_linp[offsetNumber - 1];
+       itemId = &phdr->pd_linp[offsetNumber - 1];
        (*itemId).lp_off = upper;
        (*itemId).lp_len = size;
        (*itemId).lp_flags = flags;
+
        memmove((char *) page + upper, item, size);
-       ((PageHeader) page)->pd_lower = lower;
-       ((PageHeader) page)->pd_upper = upper;
+
+       phdr->pd_lower = (LocationIndex) lower;
+       phdr->pd_upper = (LocationIndex) upper;
 
        return offsetNumber;
 }
@@ -383,79 +386,76 @@ PageRepairFragmentation(Page page, OffsetNumber *unused)
 Size
 PageGetFreeSpace(Page page)
 {
-       Size            space;
+       int                     space;
 
-       space = ((PageHeader) page)->pd_upper - ((PageHeader) page)->pd_lower;
+       /*
+        * Use signed arithmetic here so that we behave sensibly if
+        * pd_lower > pd_upper.
+        */
+       space = (int) ((PageHeader) page)->pd_upper -
+               (int) ((PageHeader) page)->pd_lower;
 
-       if (space < sizeof(ItemIdData))
+       if (space < (int) sizeof(ItemIdData))
                return 0;
        space -= sizeof(ItemIdData);    /* XXX not always appropriate */
 
-       return space;
+       return (Size) space;
 }
 
-/*
- * PageRepairFragmentation un-useful for index page cleanup because
- * of it doesn't remove line pointers. This routine could be more
- * effective but ... no time -:)
- */
-void
-IndexPageCleanup(Buffer buffer)
-{
-       Page            page = (Page) BufferGetPage(buffer);
-       ItemId          lp;
-       OffsetNumber maxoff;
-       OffsetNumber i;
-
-       maxoff = PageGetMaxOffsetNumber(page);
-       for (i = 0; i < maxoff; i++)
-       {
-               lp = ((PageHeader) page)->pd_linp + i;
-               if ((*lp).lp_flags & LP_DELETE) /* marked for deletion */
-               {
-                       PageIndexTupleDelete(page, i + 1);
-                       maxoff--;
-               }
-       }
-}
 
 /*
- *----------------------------------------------------------------
  * PageIndexTupleDelete
- *----------------------------------------------------------------
  *
- *             This routine does the work of removing a tuple from an index page.
+ * This routine does the work of removing a tuple from an index page.
+ *
+ * Unlike heap pages, we compact out the line pointer for the removed tuple.
  */
 void
 PageIndexTupleDelete(Page page, OffsetNumber offnum)
 {
-       PageHeader      phdr;
+       PageHeader      phdr = (PageHeader) page;
        char       *addr;
        ItemId          tup;
        Size            size;
-       char       *locn;
+       unsigned        offset;
        int                     nbytes;
        int                     offidx;
+       int                     nline,
+                               i;
+
+       /*
+        * As with PageRepairFragmentation, paranoia seems justified.
+        */
+       if (phdr->pd_lower < (sizeof(PageHeaderData) - sizeof(ItemIdData)) ||
+               phdr->pd_lower > phdr->pd_upper ||
+               phdr->pd_upper > phdr->pd_special ||
+               phdr->pd_special > BLCKSZ)
+               elog(ERROR, "PageIndexTupleDelete: corrupted page pointers: lower = %u, upper = %u, special = %u",
+                        phdr->pd_lower, phdr->pd_upper, phdr->pd_special);
 
-       phdr = (PageHeader) page;
+       nline = PageGetMaxOffsetNumber(page);
+       if ((int) offnum <= 0 || (int) offnum > nline)
+               elog(ERROR, "PageIndexTupleDelete: bad offnum %u", offnum);
 
        /* change offset number to offset index */
        offidx = offnum - 1;
 
        tup = PageGetItemId(page, offnum);
        size = ItemIdGetLength(tup);
-       size = MAXALIGN(size);
+       offset = ItemIdGetOffset(tup);
 
-       /* location of deleted tuple data */
-       locn = (char *) (page + ItemIdGetOffset(tup));
+       if (offset < phdr->pd_upper || (offset + size) > phdr->pd_special ||
+               offset != MAXALIGN(offset) || size != MAXALIGN(size))
+               elog(ERROR, "PageIndexTupleDelete: corrupted item pointer: offset = %u size = %u",
+                        offset, size);
 
        /*
         * First, we want to get rid of the pd_linp entry for the index tuple.
         * We copy all subsequent linp's back one slot in the array.
         */
-
        nbytes = phdr->pd_lower -
                ((char *) &phdr->pd_linp[offidx + 1] - (char *) phdr);
+
        memmove((char *) &(phdr->pd_linp[offidx]),
                        (char *) &(phdr->pd_linp[offidx + 1]),
                        nbytes);
@@ -470,52 +470,28 @@ PageIndexTupleDelete(Page page, OffsetNumber offnum)
         */
 
        /* beginning of tuple space */
-       addr = (char *) (page + phdr->pd_upper);
+       addr = (char *) page + phdr->pd_upper;
 
-       if (locn != addr)
-               memmove(addr + size, addr, (int) (locn - addr));
+       if (offset > phdr->pd_upper)
+               memmove(addr + size, addr, (int) (offset - phdr->pd_upper));
 
        /* adjust free space boundary pointers */
        phdr->pd_upper += size;
        phdr->pd_lower -= sizeof(ItemIdData);
 
-       /* finally, we need to adjust the linp entries that remain */
+       /*
+        * Finally, we need to adjust the linp entries that remain.
+        *
+        * Anything that used to be before the deleted tuple's data was moved
+        * forward by the size of the deleted tuple.
+        */
        if (!PageIsEmpty(page))
-               PageIndexTupleDeleteAdjustLinePointers(phdr, locn, size);
-}
-
-/*
- *----------------------------------------------------------------
- * PageIndexTupleDeleteAdjustLinePointers
- *----------------------------------------------------------------
- *
- *             Once the line pointers and tuple data have been shifted around
- *             on the page, we need to go down the line pointer vector and
- *             adjust pointers to reflect new locations.  Anything that used
- *             to be before the deleted tuple's data was moved forward by the
- *             size of the deleted tuple.
- *
- *             This routine does the work of adjusting the line pointers.
- *             Location is where the tuple data used to lie; size is how
- *             much space it occupied.  We assume that size has been aligned
- *             as required by the time we get here.
- *
- *             This routine should never be called on an empty page.
- */
-static void
-PageIndexTupleDeleteAdjustLinePointers(PageHeader phdr,
-                                                                          char *location,
-                                                                          Size size)
-{
-       int                     i;
-       unsigned        offset;
-
-       /* location is an index into the page... */
-       offset = (unsigned) (location - (char *) phdr);
-
-       for (i = PageGetMaxOffsetNumber((Page) phdr) - 1; i >= 0; i--)
        {
-               if (phdr->pd_linp[i].lp_off <= offset)
-                       phdr->pd_linp[i].lp_off += size;
+               nline--;                                /* there's one less than when we started */
+               for (i = nline; --i >= 0; )
+               {
+                       if (phdr->pd_linp[i].lp_off <= offset)
+                               phdr->pd_linp[i].lp_off += size;
+               }
        }
 }
index 6e75f12b3c830ba019af8234bd566b2c8aad4e37..3537d8f9e2dacf2be242e9bd1f02a3cd6609dda1 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2001, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: bufpage.h,v 1.46 2001/11/05 17:46:35 momjian Exp $
+ * $Id: bufpage.h,v 1.47 2002/01/15 22:14:17 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -310,7 +310,6 @@ typedef enum
  */
 
 extern void PageInit(Page page, Size pageSize, Size specialSize);
-extern void PageZero(Page page);
 extern OffsetNumber PageAddItem(Page page, Item item, Size size,
                        OffsetNumber offsetNumber, ItemIdFlags flags);
 extern Page PageGetTempPage(Page page, Size specialSize);
@@ -318,6 +317,5 @@ extern void PageRestoreTempPage(Page tempPage, Page oldPage);
 extern int     PageRepairFragmentation(Page page, OffsetNumber *unused);
 extern Size PageGetFreeSpace(Page page);
 extern void PageIndexTupleDelete(Page page, OffsetNumber offset);
-extern void IndexPageCleanup(Buffer buffer);
 
 #endif   /* BUFPAGE_H */