Fix cache reference leak in contrib/sepgsql.

fixup_whole_row_references() did the wrong thing with a dropped column,
resulting in a commit-time warning about a cache reference leak.

I (tgl) added a test case exercising this, but back-patched the test
only as far as v10; the patch didn't apply cleanly to 9.6 and it
didn't seem worth the trouble to adapt it.  The bug is pretty old
though, so apply the code change all the way back.

Michael Luo, with cosmetic improvements by me

Discussion: https://postgr.es/m/BYAPR08MB5606D1453D7F50E2AF4D2FD29AD80@BYAPR08MB5606.namprd08.prod.outlook.com

diff --git a/contrib/sepgsql/dml.c b/contrib/sepgsql/dml.c
index 0e9aa47eed30a21f13c56ed39513f49cc9ad7f3a..53f6f41c5c41534b2c38fc82d7f0dee649a2d800 100644 (file)
--- a/contrib/sepgsql/dml.c
+++ b/contrib/sepgsql/dml.c
@@ -30,9 +30,9 @@
 /*
  * fixup_whole_row_references
  *
- * When user reference a whole of row, it is equivalent to reference to
+ * When user references a whole-row Var, it is equivalent to referencing
  * all the user columns (not system columns). So, we need to fix up the
- * given bitmapset, if it contains a whole of the row reference.
+ * given bitmapset, if it contains a whole-row reference.
  */
 static Bitmapset *
 fixup_whole_row_references(Oid relOid, Bitmapset *columns)
@@ -43,7 +43,7 @@ fixup_whole_row_references(Oid relOid, Bitmapset *columns)
    AttrNumber  attno;
    int         index;
 
-   /* if no whole of row references, do not anything */
+   /* if no whole-row references, nothing to do */
    index = InvalidAttrNumber - FirstLowInvalidHeapAttributeNumber;
    if (!bms_is_member(index, columns))
        return columns;
@@ -55,7 +55,7 @@ fixup_whole_row_references(Oid relOid, Bitmapset *columns)
    natts = ((Form_pg_class) GETSTRUCT(tuple))->relnatts;
    ReleaseSysCache(tuple);
 
-   /* fix up the given columns */
+   /* remove bit 0 from column set, add in all the non-dropped columns */
    result = bms_copy(columns);
    result = bms_del_member(result, index);
 
@@ -65,14 +65,13 @@ fixup_whole_row_references(Oid relOid, Bitmapset *columns)
                                ObjectIdGetDatum(relOid),
                                Int16GetDatum(attno));
        if (!HeapTupleIsValid(tuple))
-           continue;
-
-       if (((Form_pg_attribute) GETSTRUCT(tuple))->attisdropped)
-           continue;
-
-       index = attno - FirstLowInvalidHeapAttributeNumber;
+           continue;           /* unexpected case, should we error? */
 
-       result = bms_add_member(result, index);
+       if (!((Form_pg_attribute) GETSTRUCT(tuple))->attisdropped)
+       {
+           index = attno - FirstLowInvalidHeapAttributeNumber;
+           result = bms_add_member(result, index);
+       }
 
        ReleaseSysCache(tuple);
    }

diff --git a/contrib/sepgsql/expected/dml.out b/contrib/sepgsql/expected/dml.out
index 31243c723b474f11b19b17283bcb98054a112e73..6d5b1c1903639e788bc8cb8456f5f1960f64d0a9 100644 (file)
--- a/contrib/sepgsql/expected/dml.out
+++ b/contrib/sepgsql/expected/dml.out
@@ -4,8 +4,9 @@
 --
 -- Setup
 --
-CREATE TABLE t1 (a int, b text);
+CREATE TABLE t1 (a int, junk int, b text);
 SECURITY LABEL ON TABLE t1 IS 'system_u:object_r:sepgsql_table_t:s0';
+ALTER TABLE t1 DROP COLUMN junk;
 INSERT INTO t1 VALUES (1, 'aaa'), (2, 'bbb'), (3, 'ccc');
 CREATE TABLE t2 (x int, y text);
 SECURITY LABEL ON TABLE t2 IS 'system_u:object_r:sepgsql_ro_table_t:s0';
@@ -13,8 +14,9 @@ INSERT INTO t2 VALUES (1, 'xxx'), (2, 'yyy'), (3, 'zzz');
 CREATE TABLE t3 (s int, t text);
 SECURITY LABEL ON TABLE t3 IS 'system_u:object_r:sepgsql_fixed_table_t:s0';
 INSERT INTO t3 VALUES (1, 'sss'), (2, 'ttt'), (3, 'uuu');
-CREATE TABLE t4 (m int, n text);
+CREATE TABLE t4 (m int, junk int, n text);
 SECURITY LABEL ON TABLE t4 IS 'system_u:object_r:sepgsql_secret_table_t:s0';
+ALTER TABLE t4 DROP COLUMN junk;
 INSERT INTO t4 VALUES (1, 'mmm'), (2, 'nnn'), (3, 'ooo');
 CREATE TABLE t5 (e text, f text, g text);
 SECURITY LABEL ON TABLE t5 IS 'system_u:object_r:sepgsql_table_t:s0';
@@ -136,6 +138,16 @@ SELECT e,f FROM t5;            -- ok
 ---+---
 (0 rows)
 
+SELECT (t1.*)::record FROM t1;     -- ok
+   t1    
+---------
+ (1,aaa)
+ (2,bbb)
+ (3,ccc)
+(3 rows)
+
+SELECT (t4.*)::record FROM t4;     -- failed
+ERROR:  SELinux: security policy violation
 ---
 -- partitioned table parent
 SELECT * FROM t1p;         -- failed

diff --git a/contrib/sepgsql/sql/dml.sql b/contrib/sepgsql/sql/dml.sql
index 19201f4b90b4a00ef8592269c8a17ad65f1ce2c0..4a47b4a3c62efd7f3c3177ace38f2e17fa3d6f06 100644 (file)
--- a/contrib/sepgsql/sql/dml.sql
+++ b/contrib/sepgsql/sql/dml.sql
@@ -5,8 +5,9 @@
 --
 -- Setup
 --
-CREATE TABLE t1 (a int, b text);
+CREATE TABLE t1 (a int, junk int, b text);
 SECURITY LABEL ON TABLE t1 IS 'system_u:object_r:sepgsql_table_t:s0';
+ALTER TABLE t1 DROP COLUMN junk;
 INSERT INTO t1 VALUES (1, 'aaa'), (2, 'bbb'), (3, 'ccc');
 
 CREATE TABLE t2 (x int, y text);
@@ -17,8 +18,9 @@ CREATE TABLE t3 (s int, t text);
 SECURITY LABEL ON TABLE t3 IS 'system_u:object_r:sepgsql_fixed_table_t:s0';
 INSERT INTO t3 VALUES (1, 'sss'), (2, 'ttt'), (3, 'uuu');
 
-CREATE TABLE t4 (m int, n text);
+CREATE TABLE t4 (m int, junk int, n text);
 SECURITY LABEL ON TABLE t4 IS 'system_u:object_r:sepgsql_secret_table_t:s0';
+ALTER TABLE t4 DROP COLUMN junk;
 INSERT INTO t4 VALUES (1, 'mmm'), (2, 'nnn'), (3, 'ooo');
 
 CREATE TABLE t5 (e text, f text, g text);
@@ -95,6 +97,8 @@ SELECT * FROM t3;         -- ok
 SELECT * FROM t4;          -- failed
 SELECT * FROM t5;          -- failed
 SELECT e,f FROM t5;            -- ok
+SELECT (t1.*)::record FROM t1;     -- ok
+SELECT (t4.*)::record FROM t4;     -- failed
 
 ---
 -- partitioned table parent