From 14e9b18fed289e483ed28daec60fdab95da9cc48 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 3 Feb 2017 11:34:41 -0500 Subject: [PATCH] In pageinspect/hashfuncs.c, avoid crashes on alignment-picky machines. On machines with MAXALIGN = 8, the payload of a bytea is not maxaligned, since it will start 4 bytes into a palloc'd value. On alignment-picky hardware, this will cause failures in accesses to 8-byte-wide values within the page. We already encountered this problem when we introduced GIN index inspection functions, and fixed it in commit 84ad68d64. Make use of the same function for hash indexes. A small difficulty is that up to now contrib/pageinspect has not shared any functions at all across files. To support that, introduce a common header file "pageinspect.h" for the module. Also, move get_page_from_raw() out of ginfuncs.c, where it didn't especially belong, and put it in rawpage.c which seems a more natural home. Per buildfarm. Discussion: https://postgr.es/m/17311.1486134714@sss.pgh.pa.us --- contrib/pageinspect/brinfuncs.c | 2 ++ contrib/pageinspect/btreefuncs.c | 2 ++ contrib/pageinspect/fsmfuncs.c | 2 ++ contrib/pageinspect/ginfuncs.c | 22 ++---------------- contrib/pageinspect/hashfuncs.c | 18 ++++----------- contrib/pageinspect/heapfuncs.c | 2 ++ contrib/pageinspect/pageinspect.h | 21 +++++++++++++++++ contrib/pageinspect/rawpage.c | 38 +++++++++++++++++++++++++++++++ 8 files changed, 73 insertions(+), 34 deletions(-) create mode 100644 contrib/pageinspect/pageinspect.h diff --git a/contrib/pageinspect/brinfuncs.c b/contrib/pageinspect/brinfuncs.c index 7b877e3d0c..2c7963ec19 100644 --- a/contrib/pageinspect/brinfuncs.c +++ b/contrib/pageinspect/brinfuncs.c @@ -9,6 +9,8 @@ */ #include "postgres.h" +#include "pageinspect.h" + #include "access/htup_details.h" #include "access/brin.h" #include "access/brin_internal.h" diff --git a/contrib/pageinspect/btreefuncs.c b/contrib/pageinspect/btreefuncs.c index 3f09d5f0a4..d50ec3a68d 100644 --- a/contrib/pageinspect/btreefuncs.c +++ b/contrib/pageinspect/btreefuncs.c @@ -27,6 +27,8 @@ #include "postgres.h" +#include "pageinspect.h" + #include "access/nbtree.h" #include "catalog/namespace.h" #include "catalog/pg_am.h" diff --git a/contrib/pageinspect/fsmfuncs.c b/contrib/pageinspect/fsmfuncs.c index 654164616b..615dab8b13 100644 --- a/contrib/pageinspect/fsmfuncs.c +++ b/contrib/pageinspect/fsmfuncs.c @@ -19,6 +19,8 @@ #include "postgres.h" +#include "pageinspect.h" + #include "funcapi.h" #include "lib/stringinfo.h" #include "miscadmin.h" diff --git a/contrib/pageinspect/ginfuncs.c b/contrib/pageinspect/ginfuncs.c index cea77d301e..993fc2d9ae 100644 --- a/contrib/pageinspect/ginfuncs.c +++ b/contrib/pageinspect/ginfuncs.c @@ -9,6 +9,8 @@ */ #include "postgres.h" +#include "pageinspect.h" + #include "access/gin.h" #include "access/gin_private.h" #include "access/htup_details.h" @@ -29,26 +31,6 @@ PG_FUNCTION_INFO_V1(gin_page_opaque_info); PG_FUNCTION_INFO_V1(gin_leafpage_items); -static Page -get_page_from_raw(bytea *raw_page) -{ - int raw_page_size; - Page page; - - raw_page_size = VARSIZE(raw_page) - VARHDRSZ; - if (raw_page_size < BLCKSZ) - ereport(ERROR, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("input page too small (%d bytes)", raw_page_size))); - - /* make a copy so that the page is properly aligned for struct access */ - page = palloc(raw_page_size); - memcpy(page, VARDATA(raw_page), raw_page_size); - - return page; -} - - Datum gin_metapage_info(PG_FUNCTION_ARGS) { diff --git a/contrib/pageinspect/hashfuncs.c b/contrib/pageinspect/hashfuncs.c index 83b469864a..49cb12e518 100644 --- a/contrib/pageinspect/hashfuncs.c +++ b/contrib/pageinspect/hashfuncs.c @@ -10,6 +10,8 @@ #include "postgres.h" +#include "pageinspect.h" + #include "access/hash.h" #include "access/htup_details.h" #include "catalog/pg_type.h" @@ -48,27 +50,15 @@ typedef struct HashPageStat /* * Verify that the given bytea contains a HASH page, or die in the attempt. - * A pointer to the page is returned. + * A pointer to a palloc'd, properly aligned copy of the page is returned. */ static Page verify_hash_page(bytea *raw_page, int flags) { - Page page; - int raw_page_size; + Page page = get_page_from_raw(raw_page); int pagetype; HashPageOpaque pageopaque; - raw_page_size = VARSIZE(raw_page) - VARHDRSZ; - - if (raw_page_size != BLCKSZ) - ereport(ERROR, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("invalid page size"), - errdetail("Expected size %d, got %d", - BLCKSZ, raw_page_size))); - - page = VARDATA(raw_page); - if (PageIsNew(page)) ereport(ERROR, (errcode(ERRCODE_INDEX_CORRUPTED), diff --git a/contrib/pageinspect/heapfuncs.c b/contrib/pageinspect/heapfuncs.c index 748b1db093..1448effb50 100644 --- a/contrib/pageinspect/heapfuncs.c +++ b/contrib/pageinspect/heapfuncs.c @@ -25,6 +25,8 @@ #include "postgres.h" +#include "pageinspect.h" + #include "access/htup_details.h" #include "funcapi.h" #include "catalog/pg_type.h" diff --git a/contrib/pageinspect/pageinspect.h b/contrib/pageinspect/pageinspect.h new file mode 100644 index 0000000000..55ca68fab3 --- /dev/null +++ b/contrib/pageinspect/pageinspect.h @@ -0,0 +1,21 @@ +/*------------------------------------------------------------------------- + * + * pageinspect.h + * Common functions for pageinspect. + * + * Copyright (c) 2017, PostgreSQL Global Development Group + * + * IDENTIFICATION + * contrib/pageinspect/pageinspect.h + * + *------------------------------------------------------------------------- + */ +#ifndef _PAGEINSPECT_H_ +#define _PAGEINSPECT_H_ + +#include "storage/bufpage.h" + +/* in rawpage.c */ +extern Page get_page_from_raw(bytea *raw_page); + +#endif /* _PAGEINSPECT_H_ */ diff --git a/contrib/pageinspect/rawpage.c b/contrib/pageinspect/rawpage.c index a2ac078d40..102f360c6f 100644 --- a/contrib/pageinspect/rawpage.c +++ b/contrib/pageinspect/rawpage.c @@ -15,6 +15,8 @@ #include "postgres.h" +#include "pageinspect.h" + #include "access/htup_details.h" #include "catalog/catalog.h" #include "catalog/namespace.h" @@ -158,6 +160,42 @@ get_raw_page_internal(text *relname, ForkNumber forknum, BlockNumber blkno) return raw_page; } + +/* + * get_page_from_raw + * + * Get a palloc'd, maxalign'ed page image from the result of get_raw_page() + * + * On machines with MAXALIGN = 8, the payload of a bytea is not maxaligned, + * since it will start 4 bytes into a palloc'd value. On alignment-picky + * machines, this will cause failures in accesses to 8-byte-wide values + * within the page. We don't need to worry if accessing only 4-byte or + * smaller fields, but when examining a struct that contains 8-byte fields, + * use this function for safety. + */ +Page +get_page_from_raw(bytea *raw_page) +{ + Page page; + int raw_page_size; + + raw_page_size = VARSIZE(raw_page) - VARHDRSZ; + + if (raw_page_size != BLCKSZ) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("invalid page size"), + errdetail("Expected %d bytes, got %d.", + BLCKSZ, raw_page_size))); + + page = palloc(raw_page_size); + + memcpy(page, VARDATA(raw_page), raw_page_size); + + return page; +} + + /* * page_header *