]> granicus.if.org Git - postgresql/commitdiff
Add defenses against running with a wrong selection of LOBLKSIZE.
authorTom Lane <tgl@sss.pgh.pa.us>
Thu, 5 Jun 2014 15:31:18 +0000 (11:31 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Thu, 5 Jun 2014 15:31:18 +0000 (11:31 -0400)
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.

src/backend/storage/large_object/inv_api.c

index 34f9e8f543d037919e778bd49588e0264494b89f..9df80b7660a1f4472e134b69f1b60cb41bcdd5e7 100644 (file)
@@ -175,13 +175,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));
-       if (VARSIZE(data) < VARHDRSZ)
-               elog(ERROR, "invalid VARSIZE(data)");
-       return (VARSIZE(data) - VARHDRSZ);
+       bytea      *datafield;
+       int                     len;
+       bool            freeit;
+
+       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;
 }
 
 
@@ -367,20 +392,14 @@ inv_getsize(LargeObjectDesc *obj_desc)
        {
                Form_pg_largeobject data;
                bytea      *datafield;
+               int                     len;
                bool            pfreeit;
 
                if (HeapTupleHasNulls(tuple))   /* paranoia */
                        elog(ERROR, "null field found in pg_largeobject");
                data = (Form_pg_largeobject) GETSTRUCT(tuple);
-               datafield = &(data->data);              /* see note at top of file */
-               pfreeit = false;
-               if (VARATT_IS_EXTENDED(datafield))
-               {
-                       datafield = (bytea *)
-                               heap_tuple_untoast_attr((struct varlena *) datafield);
-                       pfreeit = true;
-               }
-               lastbyte = data->pageno * LOBLKSIZE + getbytealen(datafield);
+               getdatafield(data, &datafield, &len, &pfreeit);
+               lastbyte = data->pageno * LOBLKSIZE + len;
                if (pfreeit)
                        pfree(datafield);
        }
@@ -495,15 +514,7 @@ inv_read(LargeObjectDesc *obj_desc, char *buf, int nbytes)
                        off = (int) (obj_desc->offset - pageoff);
                        Assert(off >= 0 && off < LOBLKSIZE);
 
-                       datafield = &(data->data);      /* see note at top of file */
-                       pfreeit = false;
-                       if (VARATT_IS_EXTENDED(datafield))
-                       {
-                               datafield = (bytea *)
-                                       heap_tuple_untoast_attr((struct varlena *) datafield);
-                               pfreeit = true;
-                       }
-                       len = getbytealen(datafield);
+                       getdatafield(data, &datafield, &len, &pfreeit);
                        if (len > off)
                        {
                                n = len - off;
@@ -622,16 +633,7 @@ inv_write(LargeObjectDesc *obj_desc, const char *buf, int nbytes)
                         *
                         * First, load old data into workbuf
                         */
-                       datafield = &(olddata->data);           /* see note at top of file */
-                       pfreeit = false;
-                       if (VARATT_IS_EXTENDED(datafield))
-                       {
-                               datafield = (bytea *)
-                                       heap_tuple_untoast_attr((struct varlena *) datafield);
-                               pfreeit = true;
-                       }
-                       len = getbytealen(datafield);
-                       Assert(len <= LOBLKSIZE);
+                       getdatafield(olddata, &datafield, &len, &pfreeit);
                        memcpy(workb, VARDATA(datafield), len);
                        if (pfreeit)
                                pfree(datafield);
@@ -807,19 +809,11 @@ inv_truncate(LargeObjectDesc *obj_desc, int len)
        if (olddata != NULL && olddata->pageno == pageno)
        {
                /* First, load old data into workbuf */
-               bytea      *datafield = &(olddata->data);               /* see note at top of
-                                                                                                                * file */
-               bool            pfreeit = false;
+               bytea      *datafield;
                int                     pagelen;
+               bool            pfreeit;
 
-               if (VARATT_IS_EXTENDED(datafield))
-               {
-                       datafield = (bytea *)
-                               heap_tuple_untoast_attr((struct varlena *) datafield);
-                       pfreeit = true;
-               }
-               pagelen = getbytealen(datafield);
-               Assert(pagelen <= LOBLKSIZE);
+               getdatafield(olddata, &datafield, &pagelen, &pfreeit);
                memcpy(workb, VARDATA(datafield), pagelen);
                if (pfreeit)
                        pfree(datafield);