Derive the lolor.node bound from the OID encoding.

A generated large object OID reserves four bits for the node id, but the
GUC accepted 0..16.  Node 16 does not fit and was silently encoded as node
0, so two nodes could generate colliding OIDs.

Move the encoding parameters into lolor.h and compute the GUC maximum from
them, so the bound cannot drift from the layout it protects.

Author: ibrarahmad

docs/lolor_release_notes.md

@@ -3,6 +3,8 @@
 ## lolor 1.4.0
 
 * **Security fix: `lo_import()` and `lo_export()` were executable by any database user.** These functions read and write files on the server as the operating system account PostgreSQL runs under, and core revokes `EXECUTE` on them from `PUBLIC`. lolor replaces them by renaming the originals to `*_orig`; an ACL belongs to a function rather than to a name, so the restriction stayed behind on the parked original while each replacement was created with the default of `EXECUTE TO PUBLIC`. Any user could therefore read an arbitrary server file with `lo_import()` or overwrite one with `lo_export()`. The replacements are now locked down at install time, and the upgrade to 1.4.0 revokes the privilege on existing installations in either the enabled or the disabled state. All versions from 1.0 through 1.3.0 are affected.
+* Fixed the `lolor.node` upper bound. The GUC accepted 0..16 while a generated OID reserves only four bits for the node id, so node 16 did not fit: the node field of every OID it generated read back as 0. The bound is now derived from the encoding (`LOLOR_MAX_NODE_ID`), giving a valid range of 0..15.
+  **If you have `lolor.node = 16` configured**, the server still starts after upgrading, but logs `16 is outside the valid range for parameter "lolor.node" (0 .. 15)` and falls back to 0. That is the node id it was effectively using already, so no OIDs change. Set it to a value in 0..15, and check for OID collisions against whichever node is genuinely 0, since the two have been sharing a node field.
 * The extension is no longer marked `trusted`. Installing lolor renames functions in `pg_catalog` for the whole database, which is not an operation a non-superuser should be able to perform.
 
 ## lolor 1.3.0

expected/lolor.out

@@ -722,6 +722,15 @@ ORDER BY 1;
 DROP EXTENSION lolor;
 NOTICE:  no lolor large objects to migrate
 --
+-- lolor.node is bounded by the OID encoding, not by an independently written
+-- constant: the low LOLOR_NODEID_BITS of a generated OID carry the node id, so
+-- 16 does not fit and was previously accepted while encoding as node 0.
+--
+SET lolor.node = 16;
+ERROR:  16 is outside the valid range for parameter "lolor.node" (0 .. 15)
+SET lolor.node = 15;
+SET lolor.node = 1;
+--
 -- 64-bit interface and page-boundary I/O.  lo_put(), lo_tell64() and
 -- lo_truncate64() had no coverage.
 --

sql/lolor.sql

@@ -309,6 +309,15 @@ WHERE r.proname IN ('lo_import', 'lo_export')
 ORDER BY 1;
 DROP EXTENSION lolor;
 
+--
+-- lolor.node is bounded by the OID encoding, not by an independently written
+-- constant: the low LOLOR_NODEID_BITS of a generated OID carry the node id, so
+-- 16 does not fit and was previously accepted while encoding as node 0.
+--
+SET lolor.node = 16;
+SET lolor.node = 15;
+SET lolor.node = 1;
+
 --
 -- 64-bit interface and page-boundary I/O.  lo_put(), lo_tell64() and
 -- lo_truncate64() had no coverage.

src/lolor.c

@@ -156,7 +156,7 @@ _PG_init(void)
 							&lolor_node_id,
 							0,
 							0,
-							16,
+							LOLOR_MAX_NODE_ID,
 							PGC_SUSET,
 							0,
 							NULL, NULL, NULL);

src/lolor.h

@@ -21,6 +21,17 @@
 #define LOLOR_LARGEOBJECT_METADATA		"pg_largeobject_metadata"
 #define LOLOR_LARGEOBJECT_METADATA_PKEY	"pg_largeobject_metadata_pkey"
 
+/*
+ * Layout of a lolor-assigned large object OID: the low LOLOR_NODEID_BITS hold
+ * lolor.node and the rest hold the generated OID, so that concurrent creation
+ * on different nodes cannot collide.  The GUC bound is derived from the
+ * encoding rather than written out separately, since the two must agree.
+ * Changing these changes the on-disk OID encoding.
+ */
+#define LOLOR_NODEID_BITS				4
+#define LOLOR_OID_BITS					28
+#define LOLOR_MAX_NODE_ID				((1 << LOLOR_NODEID_BITS) - 1)
+
 /* lolor.c */
 extern int32 lolor_node_id;
 extern Oid get_LOLOR_LargeObjectRelationId(void);

src/lolor_largeobject.c

@@ -39,10 +39,6 @@
 #define GETNEWOID_LOG_THRESHOLD 1000000
 #define GETNEWOID_LOG_MAX_INTERVAL 128000000
 
-/* Parameters to determine new unique Oid. */
-#define MAX_NODEID_BITS 4
-#define MAX_OID_BITS 28
-
 /*
  * Create a large object having the given LO identifier.
  *
@@ -204,8 +200,9 @@ LOLOR_LargeObjectExists(Oid loid)
  * LOLOR_GetNewOidWithIndex
  *		Generate a new OID that is unique within the given relation.
  *
- * The lower 4 bits contains the lolor_node_id. The 2^28 bits consist of Oid
- * returned from GetNewObjectId and adjusted to remain within the range.
+ * The low LOLOR_NODEID_BITS contain lolor_node_id; the remaining
+ * LOLOR_OID_BITS hold an Oid returned from GetNewObjectId, adjusted to remain
+ * within range.
  *
  * See comments for GetNewOidWithIndex() for more details.
  */
@@ -236,11 +233,11 @@ LOLOR_GetNewOidWithIndex(Relation relation, Oid indexId, AttrNumber oidcolumn)
 		 * Keep the range within 1..2^28. Restart from start on overflow and see
 		 * if any of the Oids are avaialbe.
 		 */
-		newOid = newOid % (1 << MAX_OID_BITS);
+		newOid = newOid % (1 << LOLOR_OID_BITS);
 		if (newOid == 0)
 			newOid = 1;
 
-		newOid = (newOid << MAX_NODEID_BITS) | lolor_node_id;
+		newOid = (newOid << LOLOR_NODEID_BITS) | lolor_node_id;
 
 		if (IsBootstrapProcessingMode())
 			return newOid;