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

@@ -10,6 +10,7 @@
   * Migration is node-local: native large objects are never replicated, so each node holds an independent set and `migrate_from_native()` migrates only the local node's objects. Run it on every node that holds native large objects, for example via `spock.replicate_ddl('SELECT lolor.migrate_from_native()')`. Migrated objects keep their original native OIDs, which are not node-encoded and can collide across nodes if different nodes hold different objects under the same OID; newly created large objects are collision-free, since new OIDs are node-encoded via `lolor.node` and checked against existing rows.
 * **Security fix: `lo_import()` and `lo_export()` were executable by any database user.** These read and write files on the server as the account PostgreSQL runs under, so core revokes `EXECUTE` on them from `PUBLIC`. lolor replaces them by renaming the originals to `*_orig`; an ACL belongs to a function rather than a name, so the restriction stayed on the parked original while each replacement got the default `EXECUTE TO PUBLIC`. Any user could read an arbitrary server file with `lo_import()` or overwrite one with `lo_export()`. The replacements are now locked down at install time, and upgrading revokes the privilege on existing installations in either state. Versions 1.0 through 1.2.2 are affected.
 * The extension is no longer marked `trusted`. Installing lolor renames functions in `pg_catalog` for the whole database, which a non-superuser should not be able to do.
+* Fixed the `lolor.node` upper bound. The GUC accepted 0..16 while a generated OID reserves 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 0..15. **If you have `lolor.node = 16` configured**, the server still starts but logs `16 is outside the valid range for parameter "lolor.node" (0 .. 15)` and falls back to 0, which is the node id it was effectively using already. Set it to a value in 0..15, and check for OID collisions against whichever node is genuinely 0.
 * Expanded test coverage: TAP tests for dump/restore, streaming and logical replication, and standby promotion; regression tests for `lo_lseek`, `lo_tell`, and `lo_truncate`.
 * Security hardening: addressed Codacy/Flawfinder warnings.
 

expected/lolor.out

@@ -721,6 +721,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

@@ -308,6 +308,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;