Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit fc576b7

Browse files
committed
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
1 parent 24d2d38 commit fc576b7

File tree

3 files changed

+30
-15
lines changed

3 files changed

+30
-15
lines changed

contrib/sepgsql/dml.c

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,9 @@
3030
/*
3131
* fixup_whole_row_references
3232
*
33-
* When user reference a whole of row, it is equivalent to reference to
33+
* When user references a whole-row Var, it is equivalent to referencing
3434
* all the user columns (not system columns). So, we need to fix up the
35-
* given bitmapset, if it contains a whole of the row reference.
35+
* given bitmapset, if it contains a whole-row reference.
3636
*/
3737
static Bitmapset *
3838
fixup_whole_row_references(Oid relOid, Bitmapset *columns)
@@ -43,7 +43,7 @@ fixup_whole_row_references(Oid relOid, Bitmapset *columns)
4343
AttrNumber attno;
4444
int index;
4545

46-
/* if no whole of row references, do not anything */
46+
/* if no whole-row references, nothing to do */
4747
index = InvalidAttrNumber - FirstLowInvalidHeapAttributeNumber;
4848
if (!bms_is_member(index, columns))
4949
return columns;
@@ -55,7 +55,7 @@ fixup_whole_row_references(Oid relOid, Bitmapset *columns)
5555
natts = ((Form_pg_class) GETSTRUCT(tuple))->relnatts;
5656
ReleaseSysCache(tuple);
5757

58-
/* fix up the given columns */
58+
/* remove bit 0 from column set, add in all the non-dropped columns */
5959
result = bms_copy(columns);
6060
result = bms_del_member(result, index);
6161

@@ -65,14 +65,13 @@ fixup_whole_row_references(Oid relOid, Bitmapset *columns)
6565
ObjectIdGetDatum(relOid),
6666
Int16GetDatum(attno));
6767
if (!HeapTupleIsValid(tuple))
68-
continue;
69-
70-
if (((Form_pg_attribute) GETSTRUCT(tuple))->attisdropped)
71-
continue;
72-
73-
index = attno - FirstLowInvalidHeapAttributeNumber;
68+
continue; /* unexpected case, should we error? */
7469

75-
result = bms_add_member(result, index);
70+
if (!((Form_pg_attribute) GETSTRUCT(tuple))->attisdropped)
71+
{
72+
index = attno - FirstLowInvalidHeapAttributeNumber;
73+
result = bms_add_member(result, index);
74+
}
7675

7776
ReleaseSysCache(tuple);
7877
}

contrib/sepgsql/expected/dml.out

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,19 @@
44
--
55
-- Setup
66
--
7-
CREATE TABLE t1 (a int, b text);
7+
CREATE TABLE t1 (a int, junk int, b text);
88
SECURITY LABEL ON TABLE t1 IS 'system_u:object_r:sepgsql_table_t:s0';
9+
ALTER TABLE t1 DROP COLUMN junk;
910
INSERT INTO t1 VALUES (1, 'aaa'), (2, 'bbb'), (3, 'ccc');
1011
CREATE TABLE t2 (x int, y text);
1112
SECURITY LABEL ON TABLE t2 IS 'system_u:object_r:sepgsql_ro_table_t:s0';
1213
INSERT INTO t2 VALUES (1, 'xxx'), (2, 'yyy'), (3, 'zzz');
1314
CREATE TABLE t3 (s int, t text);
1415
SECURITY LABEL ON TABLE t3 IS 'system_u:object_r:sepgsql_fixed_table_t:s0';
1516
INSERT INTO t3 VALUES (1, 'sss'), (2, 'ttt'), (3, 'uuu');
16-
CREATE TABLE t4 (m int, n text);
17+
CREATE TABLE t4 (m int, junk int, n text);
1718
SECURITY LABEL ON TABLE t4 IS 'system_u:object_r:sepgsql_secret_table_t:s0';
19+
ALTER TABLE t4 DROP COLUMN junk;
1820
INSERT INTO t4 VALUES (1, 'mmm'), (2, 'nnn'), (3, 'ooo');
1921
CREATE TABLE t5 (e text, f text, g text);
2022
SECURITY LABEL ON TABLE t5 IS 'system_u:object_r:sepgsql_table_t:s0';
@@ -136,6 +138,16 @@ SELECT e,f FROM t5; -- ok
136138
---+---
137139
(0 rows)
138140

141+
SELECT (t1.*)::record FROM t1; -- ok
142+
t1
143+
---------
144+
(1,aaa)
145+
(2,bbb)
146+
(3,ccc)
147+
(3 rows)
148+
149+
SELECT (t4.*)::record FROM t4; -- failed
150+
ERROR: SELinux: security policy violation
139151
---
140152
-- partitioned table parent
141153
SELECT * FROM t1p; -- failed

contrib/sepgsql/sql/dml.sql

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,9 @@
55
--
66
-- Setup
77
--
8-
CREATE TABLE t1 (a int, b text);
8+
CREATE TABLE t1 (a int, junk int, b text);
99
SECURITY LABEL ON TABLE t1 IS 'system_u:object_r:sepgsql_table_t:s0';
10+
ALTER TABLE t1 DROP COLUMN junk;
1011
INSERT INTO t1 VALUES (1, 'aaa'), (2, 'bbb'), (3, 'ccc');
1112

1213
CREATE TABLE t2 (x int, y text);
@@ -17,8 +18,9 @@ CREATE TABLE t3 (s int, t text);
1718
SECURITY LABEL ON TABLE t3 IS 'system_u:object_r:sepgsql_fixed_table_t:s0';
1819
INSERT INTO t3 VALUES (1, 'sss'), (2, 'ttt'), (3, 'uuu');
1920

20-
CREATE TABLE t4 (m int, n text);
21+
CREATE TABLE t4 (m int, junk int, n text);
2122
SECURITY LABEL ON TABLE t4 IS 'system_u:object_r:sepgsql_secret_table_t:s0';
23+
ALTER TABLE t4 DROP COLUMN junk;
2224
INSERT INTO t4 VALUES (1, 'mmm'), (2, 'nnn'), (3, 'ooo');
2325

2426
CREATE TABLE t5 (e text, f text, g text);
@@ -95,6 +97,8 @@ SELECT * FROM t3; -- ok
9597
SELECT * FROM t4; -- failed
9698
SELECT * FROM t5; -- failed
9799
SELECT e,f FROM t5; -- ok
100+
SELECT (t1.*)::record FROM t1; -- ok
101+
SELECT (t4.*)::record FROM t4; -- failed
98102

99103
---
100104
-- partitioned table parent

0 commit comments

Comments
 (0)