Avoid direct C access to possibly-null pg_subscription_rel.srsublsn.

This coding technique is unsafe, since we'd be accessing off the end
of the tuple if the field is null.  SIGSEGV is pretty improbable, but
perhaps not impossible.  Also, returning garbage for the LSN doesn't
seem like a great idea, even if callers aren't looking at it today.

Also update docs to point out explicitly that
pg_subscription.subslotname and pg_subscription_rel.srsublsn
can be null.

Perhaps we should mark these two fields BKI_FORCE_NULL, so that
they'd be correctly labeled in databases that are initdb'd in the
future.  But we can't force that for existing databases, and on
balance it's not too clear that having a mix of different catalog
contents in the field would be wise.

Apply to v10 (where this code came in) through v12.  Already
fixed in v13 and HEAD.

Discussion: https://postgr.es/m/732838.1595278439@sss.pgh.pa.us

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 9f2273b583394000ad5fdc89e65f71519ec0b6f4..f314f93051dd98942d66464b82cc1add466b0c5d 100644 (file)
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -6578,8 +6578,9 @@ SCRAM-SHA-256$<iteration count>:<salt><
       subslotname
       name
       
-      Name of the replication slot in the upstream database. Also used
-       for local replication origin name.
+      Name of the replication slot in the upstream database (also used
+       for the local replication origin name);
+       null represents NONE
      
 
      
@@ -6661,7 +6662,9 @@ SCRAM-SHA-256$<iteration count>:<salt><
       pg_lsn
       
       
-       End LSN for s and r states.
+       Remote LSN of the state change used for synchronization coordination
+       when in s or r states,
+       otherwise null
       
      
     

diff --git a/src/backend/catalog/pg_subscription.c b/src/backend/catalog/pg_subscription.c
index fb53d71cd6606167636a831ad0756c226da670d3..e241a290eeb9150a988cf6dc90d1844346b5a05d 100644 (file)
--- a/src/backend/catalog/pg_subscription.c
+++ b/src/backend/catalog/pg_subscription.c
@@ -442,13 +442,20 @@ GetSubscriptionRelations(Oid subid)
    {
        Form_pg_subscription_rel subrel;
        SubscriptionRelState *relstate;
+       Datum       d;
+       bool        isnull;
 
        subrel = (Form_pg_subscription_rel) GETSTRUCT(tup);
 
        relstate = (SubscriptionRelState *) palloc(sizeof(SubscriptionRelState));
        relstate->relid = subrel->srrelid;
        relstate->state = subrel->srsubstate;
-       relstate->lsn = subrel->srsublsn;
+       d = SysCacheGetAttr(SUBSCRIPTIONRELMAP, tup,
+                           Anum_pg_subscription_rel_srsublsn, &isnull);
+       if (isnull)
+           relstate->lsn = InvalidXLogRecPtr;
+       else
+           relstate->lsn = DatumGetLSN(d);
 
        res = lappend(res, relstate);
    }
@@ -494,13 +501,20 @@ GetSubscriptionNotReadyRelations(Oid subid)
    {
        Form_pg_subscription_rel subrel;
        SubscriptionRelState *relstate;
+       Datum       d;
+       bool        isnull;
 
        subrel = (Form_pg_subscription_rel) GETSTRUCT(tup);
 
        relstate = (SubscriptionRelState *) palloc(sizeof(SubscriptionRelState));
        relstate->relid = subrel->srrelid;
        relstate->state = subrel->srsubstate;
-       relstate->lsn = subrel->srsublsn;
+       d = SysCacheGetAttr(SUBSCRIPTIONRELMAP, tup,
+                           Anum_pg_subscription_rel_srsublsn, &isnull);
+       if (isnull)
+           relstate->lsn = InvalidXLogRecPtr;
+       else
+           relstate->lsn = DatumGetLSN(d);
 
        res = lappend(res, relstate);
    }

diff --git a/src/include/catalog/pg_subscription_rel.h b/src/include/catalog/pg_subscription_rel.h
index 991ca9d552abe0668d6a7df2443ae8597a8e8b47..fc2c1a75a54de6452d92cb8e5a758e3804cd02e6 100644 (file)
--- a/src/include/catalog/pg_subscription_rel.h
+++ b/src/include/catalog/pg_subscription_rel.h
@@ -31,8 +31,17 @@ CATALOG(pg_subscription_rel,6102) BKI_WITHOUT_OIDS
    Oid         srsubid;        /* Oid of subscription */
    Oid         srrelid;        /* Oid of relation */
    char        srsubstate;     /* state of the relation in subscription */
-   pg_lsn      srsublsn;       /* remote lsn of the state change used for
-                                * synchronization coordination */
+
+   /*
+    * Although srsublsn is a fixed-width type, it is allowed to be NULL, so
+    * we prevent direct C code access to it just as for a varlena field.
+    */
+#ifdef CATALOG_VARLEN          /* variable-length fields start here */
+
+   pg_lsn      srsublsn;       /* remote LSN of the state change used for
+                                * synchronization coordination, or NULL if
+                                * not valid */
+#endif
 } FormData_pg_subscription_rel;
 
 typedef FormData_pg_subscription_rel *Form_pg_subscription_rel;