Commit 5f93c378 authored by Tom Lane's avatar Tom Lane

Add defenses against running with a wrong selection of LOBLKSIZE.

It's critical that the backend's idea of LOBLKSIZE match the way data has
actually been divided up in pg_largeobject.  While we don't provide any
direct way to adjust that value, doing so is a one-line source code change
and various people have expressed interest recently in changing it.  So,
just as with TOAST_MAX_CHUNK_SIZE, it seems prudent to record the value in
pg_control and cross-check that the backend's compiled-in setting matches
the on-disk data.

Also tweak the code in inv_api.c so that fetches from pg_largeobject
explicitly verify that the length of the data field is not more than
LOBLKSIZE.  Formerly we just had Asserts() for that, which is no protection
at all in production builds.  In some of the call sites an overlength data
value would translate directly to a security-relevant stack clobber, so it
seems worth one extra runtime comparison to be sure.

In the back branches, we can't change the contents of pg_control; but we
can still make the extra checks in inv_api.c, which will offer some amount
of protection against running with the wrong value of LOBLKSIZE.
parent f0c10856
...@@ -49,6 +49,7 @@ ...@@ -49,6 +49,7 @@
#include "storage/bufmgr.h" #include "storage/bufmgr.h"
#include "storage/fd.h" #include "storage/fd.h"
#include "storage/ipc.h" #include "storage/ipc.h"
#include "storage/large_object.h"
#include "storage/latch.h" #include "storage/latch.h"
#include "storage/pmsignal.h" #include "storage/pmsignal.h"
#include "storage/predicate.h" #include "storage/predicate.h"
...@@ -4352,6 +4353,7 @@ WriteControlFile(void) ...@@ -4352,6 +4353,7 @@ WriteControlFile(void)
ControlFile->indexMaxKeys = INDEX_MAX_KEYS; ControlFile->indexMaxKeys = INDEX_MAX_KEYS;
ControlFile->toast_max_chunk_size = TOAST_MAX_CHUNK_SIZE; ControlFile->toast_max_chunk_size = TOAST_MAX_CHUNK_SIZE;
ControlFile->loblksize = LOBLKSIZE;
#ifdef HAVE_INT64_TIMESTAMP #ifdef HAVE_INT64_TIMESTAMP
ControlFile->enableIntTimes = true; ControlFile->enableIntTimes = true;
...@@ -4545,6 +4547,13 @@ ReadControlFile(void) ...@@ -4545,6 +4547,13 @@ ReadControlFile(void)
" but the server was compiled with TOAST_MAX_CHUNK_SIZE %d.", " but the server was compiled with TOAST_MAX_CHUNK_SIZE %d.",
ControlFile->toast_max_chunk_size, (int) TOAST_MAX_CHUNK_SIZE), ControlFile->toast_max_chunk_size, (int) TOAST_MAX_CHUNK_SIZE),
errhint("It looks like you need to recompile or initdb."))); errhint("It looks like you need to recompile or initdb.")));
if (ControlFile->loblksize != LOBLKSIZE)
ereport(FATAL,
(errmsg("database files are incompatible with server"),
errdetail("The database cluster was initialized with LOBLKSIZE %d,"
" but the server was compiled with LOBLKSIZE %d.",
ControlFile->loblksize, (int) LOBLKSIZE),
errhint("It looks like you need to recompile or initdb.")));
#ifdef HAVE_INT64_TIMESTAMP #ifdef HAVE_INT64_TIMESTAMP
if (ControlFile->enableIntTimes != true) if (ControlFile->enableIntTimes != true)
......
...@@ -173,13 +173,38 @@ myLargeObjectExists(Oid loid, Snapshot snapshot) ...@@ -173,13 +173,38 @@ myLargeObjectExists(Oid loid, Snapshot snapshot)
} }
static int32 /*
getbytealen(bytea *data) * Extract data field from a pg_largeobject tuple, detoasting if needed
* and verifying that the length is sane. Returns data pointer (a bytea *),
* data length, and an indication of whether to pfree the data pointer.
*/
static void
getdatafield(Form_pg_largeobject tuple,
bytea **pdatafield,
int *plen,
bool *pfreeit)
{ {
Assert(!VARATT_IS_EXTENDED(data)); bytea *datafield;
if (VARSIZE(data) < VARHDRSZ) int len;
elog(ERROR, "invalid VARSIZE(data)"); bool freeit;
return (VARSIZE(data) - VARHDRSZ);
datafield = &(tuple->data); /* see note at top of file */
freeit = false;
if (VARATT_IS_EXTENDED(datafield))
{
datafield = (bytea *)
heap_tuple_untoast_attr((struct varlena *) datafield);
freeit = true;
}
len = VARSIZE(datafield) - VARHDRSZ;
if (len < 0 || len > LOBLKSIZE)
ereport(ERROR,
(errcode(ERRCODE_DATA_CORRUPTED),
errmsg("pg_largeobject entry for OID %u, page %d has invalid data field size %d",
tuple->loid, tuple->pageno, len)));
*pdatafield = datafield;
*plen = len;
*pfreeit = freeit;
} }
...@@ -366,20 +391,14 @@ inv_getsize(LargeObjectDesc *obj_desc) ...@@ -366,20 +391,14 @@ inv_getsize(LargeObjectDesc *obj_desc)
{ {
Form_pg_largeobject data; Form_pg_largeobject data;
bytea *datafield; bytea *datafield;
int len;
bool pfreeit; bool pfreeit;
if (HeapTupleHasNulls(tuple)) /* paranoia */ if (HeapTupleHasNulls(tuple)) /* paranoia */
elog(ERROR, "null field found in pg_largeobject"); elog(ERROR, "null field found in pg_largeobject");
data = (Form_pg_largeobject) GETSTRUCT(tuple); data = (Form_pg_largeobject) GETSTRUCT(tuple);
datafield = &(data->data); /* see note at top of file */ getdatafield(data, &datafield, &len, &pfreeit);
pfreeit = false; lastbyte = (uint64) data->pageno * LOBLKSIZE + len;
if (VARATT_IS_EXTENDED(datafield))
{
datafield = (bytea *)
heap_tuple_untoast_attr((struct varlena *) datafield);
pfreeit = true;
}
lastbyte = (uint64) data->pageno * LOBLKSIZE + getbytealen(datafield);
if (pfreeit) if (pfreeit)
pfree(datafield); pfree(datafield);
} }
...@@ -506,15 +525,7 @@ inv_read(LargeObjectDesc *obj_desc, char *buf, int nbytes) ...@@ -506,15 +525,7 @@ inv_read(LargeObjectDesc *obj_desc, char *buf, int nbytes)
off = (int) (obj_desc->offset - pageoff); off = (int) (obj_desc->offset - pageoff);
Assert(off >= 0 && off < LOBLKSIZE); Assert(off >= 0 && off < LOBLKSIZE);
datafield = &(data->data); /* see note at top of file */ getdatafield(data, &datafield, &len, &pfreeit);
pfreeit = false;
if (VARATT_IS_EXTENDED(datafield))
{
datafield = (bytea *)
heap_tuple_untoast_attr((struct varlena *) datafield);
pfreeit = true;
}
len = getbytealen(datafield);
if (len > off) if (len > off)
{ {
n = len - off; n = len - off;
...@@ -630,16 +641,7 @@ inv_write(LargeObjectDesc *obj_desc, const char *buf, int nbytes) ...@@ -630,16 +641,7 @@ inv_write(LargeObjectDesc *obj_desc, const char *buf, int nbytes)
* *
* First, load old data into workbuf * First, load old data into workbuf
*/ */
datafield = &(olddata->data); /* see note at top of file */ getdatafield(olddata, &datafield, &len, &pfreeit);
pfreeit = false;
if (VARATT_IS_EXTENDED(datafield))
{
datafield = (bytea *)
heap_tuple_untoast_attr((struct varlena *) datafield);
pfreeit = true;
}
len = getbytealen(datafield);
Assert(len <= LOBLKSIZE);
memcpy(workb, VARDATA(datafield), len); memcpy(workb, VARDATA(datafield), len);
if (pfreeit) if (pfreeit)
pfree(datafield); pfree(datafield);
...@@ -815,19 +817,11 @@ inv_truncate(LargeObjectDesc *obj_desc, int64 len) ...@@ -815,19 +817,11 @@ inv_truncate(LargeObjectDesc *obj_desc, int64 len)
if (olddata != NULL && olddata->pageno == pageno) if (olddata != NULL && olddata->pageno == pageno)
{ {
/* First, load old data into workbuf */ /* First, load old data into workbuf */
bytea *datafield = &(olddata->data); /* see note at top of bytea *datafield;
* file */
bool pfreeit = false;
int pagelen; int pagelen;
bool pfreeit;
if (VARATT_IS_EXTENDED(datafield)) getdatafield(olddata, &datafield, &pagelen, &pfreeit);
{
datafield = (bytea *)
heap_tuple_untoast_attr((struct varlena *) datafield);
pfreeit = true;
}
pagelen = getbytealen(datafield);
Assert(pagelen <= LOBLKSIZE);
memcpy(workb, VARDATA(datafield), pagelen); memcpy(workb, VARDATA(datafield), pagelen);
if (pfreeit) if (pfreeit)
pfree(datafield); pfree(datafield);
......
...@@ -287,6 +287,8 @@ main(int argc, char *argv[]) ...@@ -287,6 +287,8 @@ main(int argc, char *argv[])
ControlFile.indexMaxKeys); ControlFile.indexMaxKeys);
printf(_("Maximum size of a TOAST chunk: %u\n"), printf(_("Maximum size of a TOAST chunk: %u\n"),
ControlFile.toast_max_chunk_size); ControlFile.toast_max_chunk_size);
printf(_("Size of a large-object chunk: %u\n"),
ControlFile.loblksize);
printf(_("Date/time type storage: %s\n"), printf(_("Date/time type storage: %s\n"),
(ControlFile.enableIntTimes ? _("64-bit integers") : _("floating-point numbers"))); (ControlFile.enableIntTimes ? _("64-bit integers") : _("floating-point numbers")));
printf(_("Float4 argument passing: %s\n"), printf(_("Float4 argument passing: %s\n"),
......
...@@ -52,6 +52,7 @@ ...@@ -52,6 +52,7 @@
#include "catalog/catversion.h" #include "catalog/catversion.h"
#include "catalog/pg_control.h" #include "catalog/pg_control.h"
#include "common/fe_memutils.h" #include "common/fe_memutils.h"
#include "storage/large_object.h"
#include "pg_getopt.h" #include "pg_getopt.h"
...@@ -536,6 +537,7 @@ GuessControlValues(void) ...@@ -536,6 +537,7 @@ GuessControlValues(void)
ControlFile.nameDataLen = NAMEDATALEN; ControlFile.nameDataLen = NAMEDATALEN;
ControlFile.indexMaxKeys = INDEX_MAX_KEYS; ControlFile.indexMaxKeys = INDEX_MAX_KEYS;
ControlFile.toast_max_chunk_size = TOAST_MAX_CHUNK_SIZE; ControlFile.toast_max_chunk_size = TOAST_MAX_CHUNK_SIZE;
ControlFile.loblksize = LOBLKSIZE;
#ifdef HAVE_INT64_TIMESTAMP #ifdef HAVE_INT64_TIMESTAMP
ControlFile.enableIntTimes = true; ControlFile.enableIntTimes = true;
#else #else
...@@ -620,6 +622,8 @@ PrintControlValues(bool guessed) ...@@ -620,6 +622,8 @@ PrintControlValues(bool guessed)
ControlFile.indexMaxKeys); ControlFile.indexMaxKeys);
printf(_("Maximum size of a TOAST chunk: %u\n"), printf(_("Maximum size of a TOAST chunk: %u\n"),
ControlFile.toast_max_chunk_size); ControlFile.toast_max_chunk_size);
printf(_("Size of a large-object chunk: %u\n"),
ControlFile.loblksize);
printf(_("Date/time type storage: %s\n"), printf(_("Date/time type storage: %s\n"),
(ControlFile.enableIntTimes ? _("64-bit integers") : _("floating-point numbers"))); (ControlFile.enableIntTimes ? _("64-bit integers") : _("floating-point numbers")));
printf(_("Float4 argument passing: %s\n"), printf(_("Float4 argument passing: %s\n"),
......
...@@ -21,7 +21,7 @@ ...@@ -21,7 +21,7 @@
/* Version identifier for this pg_control format */ /* Version identifier for this pg_control format */
#define PG_CONTROL_VERSION 941 #define PG_CONTROL_VERSION 942
/* /*
* Body of CheckPoint XLOG records. This is declared here because we keep * Body of CheckPoint XLOG records. This is declared here because we keep
...@@ -207,6 +207,7 @@ typedef struct ControlFileData ...@@ -207,6 +207,7 @@ typedef struct ControlFileData
uint32 indexMaxKeys; /* max number of columns in an index */ uint32 indexMaxKeys; /* max number of columns in an index */
uint32 toast_max_chunk_size; /* chunk size in TOAST tables */ uint32 toast_max_chunk_size; /* chunk size in TOAST tables */
uint32 loblksize; /* chunk size in pg_largeobject */
/* flag indicating internal format of timestamp, interval, time */ /* flag indicating internal format of timestamp, interval, time */
bool enableIntTimes; /* int64 storage enabled? */ bool enableIntTimes; /* int64 storage enabled? */
......
...@@ -66,6 +66,8 @@ typedef struct LargeObjectDesc ...@@ -66,6 +66,8 @@ typedef struct LargeObjectDesc
* Also, it seems to be a smart move to make the page size be a power of 2, * Also, it seems to be a smart move to make the page size be a power of 2,
* since clients will often be written to send data in power-of-2 blocks. * since clients will often be written to send data in power-of-2 blocks.
* This avoids unnecessary tuple updates caused by partial-page writes. * This avoids unnecessary tuple updates caused by partial-page writes.
*
* NB: Changing LOBLKSIZE requires an initdb.
*/ */
#define LOBLKSIZE (BLCKSZ / 4) #define LOBLKSIZE (BLCKSZ / 4)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment