On Wed, Apr 5, 2017 at 1:22 PM, Mike Palmiotto
<mike.palmio...@crunchydata.com> wrote:
> On Wed, Apr 5, 2017 at 1:19 PM, Tom Lane <t...@sss.pgh.pa.us> wrote:
>> Peter Eisentraut <peter.eisentr...@2ndquadrant.com> writes:
>>> On 4/5/17 12:04, Tom Lane wrote:
>>>> Conclusion: Fedora's gcc is playing fast and loose somehow with the
>>>> command "#include <stdbool.h>"; that does not include the file
>>>> you'd think it does, it does something magic inside the compiler.
>>>> The magic evidently includes not complaining about duplicate macro
>>>> definitions for true and false.
>>
>>> Perhaps -Wsystem-headers would change the outcome in your case.
>>
>> Hah, you're right: with that,
>>
>> In file included from /usr/include/selinux/label.h:9:0,
>> from label.c:40:
>> /usr/lib/gcc/x86_64-redhat-linux/6.3.1/include/stdbool.h:34:0: warning:
>> "true" redefined
>> #define true 1
>>
>> In file included from ../../src/include/postgres.h:47:0,
>> from label.c:11:
>> ../../src/include/c.h:206:0: note: this is the location of the previous
>> definition
>> #define true ((bool) 1)
>>
>> and likewise for "false". So that mystery is explained.
>>
>> I stand by my previous patch suggestion, except that we can replace
>> the parenthetical comment with something like "(We don't care if
>> <stdbool.h> redefines "true"/"false"; those are close enough.)".
>>
>
> Sounds good. Updated patchset will include that verbiage, along with
> some regression tests for partitioned tables.
Looks like the label regression test is failing even without these changes.
Weirdly enough, this only happens in one place:
84 SELECT objtype, objname, label FROM pg_seclabels
85 WHERE provider = 'selinux' AND objtype = 'column' AND (objname
like 't3.%' OR objname like 't4.%');
I haven't figured out why this may be (yet), but it seems like we
shouldn't assume the order of output from a SELECT. As I understand
it, the order of the output is subject to change with changes to the
planner/configuration.
I'm going to hold the partition table regression changes for a
separate patch and include some ORDER BY fixes. Will post tomorrow
In the meantime, attached are the latest and greatest patches.
Thanks,
--
Mike Palmiotto
Software Engineer
Crunchy Data Solutions
https://crunchydata.com
From 50969159f0fd7c93a15bfb7b9046512399d0b099 Mon Sep 17 00:00:00 2001
From: Mike Palmiotto <mike.palmio...@crunchydata.com>
Date: Wed, 29 Mar 2017 14:59:37 +0000
Subject: [PATCH 3/3] Add partitioned table support to sepgsql
Account for RELKIND_PARTITIONED_RELATIONS in sepgsql and treat the objects like
regular relations. This allows for proper object_access hook behavior for
partitioned tables.
---
contrib/sepgsql/label.c | 3 ++-
contrib/sepgsql/relation.c | 30 +++++++++++++++++++++---------
2 files changed, 23 insertions(+), 10 deletions(-)
diff --git a/contrib/sepgsql/label.c b/contrib/sepgsql/label.c
index f7d79ab..1a1ec57 100644
--- a/contrib/sepgsql/label.c
+++ b/contrib/sepgsql/label.c
@@ -789,7 +789,8 @@ exec_object_restorecon(struct selabel_handle * sehnd, Oid catalogId)
case RelationRelationId:
relForm = (Form_pg_class) GETSTRUCT(tuple);
- if (relForm->relkind == RELKIND_RELATION)
+ if (relForm->relkind == RELKIND_RELATION ||
+ relForm->relkind == RELKIND_PARTITIONED_TABLE)
objtype = SELABEL_DB_TABLE;
else if (relForm->relkind == RELKIND_SEQUENCE)
objtype = SELABEL_DB_SEQUENCE;
diff --git a/contrib/sepgsql/relation.c b/contrib/sepgsql/relation.c
index b794abe..0d8905c 100644
--- a/contrib/sepgsql/relation.c
+++ b/contrib/sepgsql/relation.c
@@ -54,12 +54,14 @@ sepgsql_attribute_post_create(Oid relOid, AttrNumber attnum)
ObjectAddress object;
Form_pg_attribute attForm;
StringInfoData audit_name;
+ char relkind;
/*
* Only attributes within regular relation have individual security
* labels.
*/
- if (get_rel_relkind(relOid) != RELKIND_RELATION)
+ relkind = get_rel_relkind(relOid);
+ if (relkind != RELKIND_RELATION && relkind != RELKIND_PARTITIONED_TABLE)
return;
/*
@@ -135,8 +137,10 @@ sepgsql_attribute_drop(Oid relOid, AttrNumber attnum)
{
ObjectAddress object;
char *audit_name;
+ char relkind;
- if (get_rel_relkind(relOid) != RELKIND_RELATION)
+ relkind = get_rel_relkind(relOid);
+ if (relkind != RELKIND_RELATION && relkind != RELKIND_PARTITIONED_TABLE)
return;
/*
@@ -167,8 +171,11 @@ sepgsql_attribute_relabel(Oid relOid, AttrNumber attnum,
{
ObjectAddress object;
char *audit_name;
+ char relkind;
- if (get_rel_relkind(relOid) != RELKIND_RELATION)
+ relkind = get_rel_relkind(relOid);
+
+ if (relkind != RELKIND_RELATION && relkind != RELKIND_PARTITIONED_TABLE)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("cannot set security label on non-regular columns")));
@@ -209,8 +216,11 @@ sepgsql_attribute_setattr(Oid relOid, AttrNumber attnum)
{
ObjectAddress object;
char *audit_name;
+ char relkind;
+
+ relkind = get_rel_relkind(relOid);
- if (get_rel_relkind(relOid) != RELKIND_RELATION)
+ if (relkind != RELKIND_RELATION && relkind != RELKIND_PARTITIONED_TABLE)
return;
/*
@@ -290,6 +300,7 @@ sepgsql_relation_post_create(Oid relOid)
switch (classForm->relkind)
{
+ case RELKIND_PARTITIONED_TABLE:
case RELKIND_RELATION:
tclass = SEPG_CLASS_DB_TABLE;
break;
@@ -341,10 +352,10 @@ sepgsql_relation_post_create(Oid relOid)
SetSecurityLabel(&object, SEPGSQL_LABEL_TAG, rcontext);
/*
- * We also assigns a default security label on columns of the new regular
- * tables.
+ * We also assign a default security label on columns of a new table.
*/
- if (classForm->relkind == RELKIND_RELATION)
+ if (classForm->relkind == RELKIND_RELATION ||
+ classForm->relkind == RELKIND_PARTITIONED_TABLE)
{
Relation arel;
ScanKeyData akey;
@@ -419,6 +430,7 @@ sepgsql_relation_drop(Oid relOid)
relkind = get_rel_relkind(relOid);
switch (relkind)
{
+ case RELKIND_PARTITIONED_TABLE:
case RELKIND_RELATION:
tclass = SEPG_CLASS_DB_TABLE;
break;
@@ -479,7 +491,7 @@ sepgsql_relation_drop(Oid relOid)
/*
* check db_column:{drop} permission
*/
- if (relkind == RELKIND_RELATION)
+ if (relkind == RELKIND_RELATION || relkind == RELKIND_PARTITIONED_TABLE)
{
Form_pg_attribute attForm;
CatCList *attrList;
@@ -525,7 +537,7 @@ sepgsql_relation_relabel(Oid relOid, const char *seclabel)
uint16_t tclass = 0;
relkind = get_rel_relkind(relOid);
- if (relkind == RELKIND_RELATION)
+ if (relkind == RELKIND_RELATION || relkind == RELKIND_PARTITIONED_TABLE)
tclass = SEPG_CLASS_DB_TABLE;
else if (relkind == RELKIND_SEQUENCE)
tclass = SEPG_CLASS_DB_SEQUENCE;
--
1.8.3.1
From b54a3ee6a52ea6766b958372827aad4650859ccd Mon Sep 17 00:00:00 2001
From: Mike Palmiotto <mike.palmio...@crunchydata.com>
Date: Wed, 5 Apr 2017 14:27:01 +0000
Subject: [PATCH 1/3] Deal with stdbool re-definition of bool
selinux/label.h includes stdbool.h, which redefines the bool type and results in
a warning: assignment from incompatible pointer type for sepgsql_fmgr_hook. Make
sure we clean up the bool definition after label.h is included.
---
contrib/sepgsql/label.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/contrib/sepgsql/label.c b/contrib/sepgsql/label.c
index 1a8f884..f7d79ab 100644
--- a/contrib/sepgsql/label.c
+++ b/contrib/sepgsql/label.c
@@ -10,6 +10,18 @@
*/
#include "postgres.h"
+#include <selinux/label.h>
+
+/*
+ * selinux/label.h includes stdbool.h, which #defines bool. This #define
+ * obscures the postgres definition of bool from c.h. #undef here to keep the
+ * postgres definition. This is not necessary for true/false re-definitions.
+ * Those are close enough in stdbool.h.
+ */
+#ifdef bool
+#undef bool
+#endif
+
#include "access/heapam.h"
#include "access/htup_details.h"
#include "access/genam.h"
@@ -37,8 +49,6 @@
#include "sepgsql.h"
-#include <selinux/label.h>
-
/*
* Saved hook entries (if stacked)
*/
--
1.8.3.1
From 207e8180e140efd5f93a3d52332d7b718a09bf34 Mon Sep 17 00:00:00 2001
From: Mike Palmiotto <mike.palmio...@crunchydata.com>
Date: Wed, 5 Apr 2017 14:31:25 +0000
Subject: [PATCH 2/3] Silence "maybe uninitialized" sepgsql warnings
sepgsql throws compiler warnings due to possibly uninitialized tclass in code
paths for indexes when compiling with -Og. Initialize tclass to 0 to silence
these warnings.
---
contrib/sepgsql/relation.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/contrib/sepgsql/relation.c b/contrib/sepgsql/relation.c
index ab98a9b..b794abe 100644
--- a/contrib/sepgsql/relation.c
+++ b/contrib/sepgsql/relation.c
@@ -243,7 +243,7 @@ sepgsql_relation_post_create(Oid relOid)
HeapTuple tuple;
Form_pg_class classForm;
ObjectAddress object;
- uint16 tclass;
+ uint16_t tclass = 0;
char *scontext; /* subject */
char *tcontext; /* schema */
char *rcontext; /* relation */
@@ -413,7 +413,7 @@ sepgsql_relation_drop(Oid relOid)
{
ObjectAddress object;
char *audit_name;
- uint16_t tclass;
+ uint16_t tclass = 0;
char relkind;
relkind = get_rel_relkind(relOid);
@@ -580,7 +580,7 @@ sepgsql_relation_setattr(Oid relOid)
Form_pg_class newform;
ObjectAddress object;
char *audit_name;
- uint16_t tclass;
+ uint16_t tclass = 0;
switch (get_rel_relkind(relOid))
{
--
1.8.3.1
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
