[PATCH 1/6] libsepol: use IN6ADDR_ANY_INIT to initialize IPv6 addresses

[PATCH 1/6] libsepol: use IN6ADDR_ANY_INIT to initialize IPv6 addresses

[Nicolas Iooss]

When compiling libsepol with clang and some warning flags, the compiler
complains about the way IPv6 addresses are initialized:

    kernel_to_cil.c:2795:35: error: suggest braces around initialization
    of subobject [-Werror,-Wmissing-braces]
            struct in6_addr subnet_prefix = {0};
                                             ^
                                             {}

When replacing the initializer as suggested, gcc 4.8.4 complains:

    kernel_to_cil.c: In function ‘write_selinux_ibpkey_rules_to_cil’:
    kernel_to_cil.c:2795:9: error: missing initializer for field
    ‘__in6_u’ of ‘struct in6_addr’ [-Werror=missing-field-initializers]
      struct in6_addr subnet_prefix = {};
             ^

Thankfully netinet/in.h provides a macro to initialize struct in6_addr
variables:

    #define IN6ADDR_ANY_INIT { { { 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0 } } }

Both clang and gcc no longer report warnings when using this macro.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
---
 libsepol/src/kernel_to_cil.c  | 2 +-
 libsepol/src/kernel_to_conf.c | 2 +-
 libsepol/src/module_to_cil.c  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/libsepol/src/kernel_to_cil.c b/libsepol/src/kernel_to_cil.c
index f1905a958ec0..0055c2386695 100644
--- a/libsepol/src/kernel_to_cil.c
+++ b/libsepol/src/kernel_to_cil.c
@@ -2788,7 +2788,7 @@ static int write_selinux_ibpkey_rules_to_cil(FILE *out, struct policydb *pdb)
 {
 	struct ocontext *ibpkeycon;
 	char subnet_prefix_str[INET6_ADDRSTRLEN];
-	struct in6_addr subnet_prefix = {0};
+	struct in6_addr subnet_prefix = IN6ADDR_ANY_INIT;
 	uint16_t low;
 	uint16_t high;
 	char low_high_str[44]; /* 2^64 <= 20 digits so "(low high)" <= 44 chars */
diff --git a/libsepol/src/kernel_to_conf.c b/libsepol/src/kernel_to_conf.c
index a74873f01687..95aa92fc8c26 100644
--- a/libsepol/src/kernel_to_conf.c
+++ b/libsepol/src/kernel_to_conf.c
@@ -2649,7 +2649,7 @@ static int write_selinux_ibpkey_rules_to_conf(FILE *out, struct policydb *pdb)
 {
 	struct ocontext *ibpkeycon;
 	char subnet_prefix_str[INET6_ADDRSTRLEN];
-	struct in6_addr subnet_prefix = {0};
+	struct in6_addr subnet_prefix = IN6ADDR_ANY_INIT;
 	uint16_t low;
 	uint16_t high;
 	char low_high_str[44]; /* 2^64 <= 20 digits so "low-high" <= 44 chars */
diff --git a/libsepol/src/module_to_cil.c b/libsepol/src/module_to_cil.c
index 619a48f8c7b6..15b58a7aacee 100644
--- a/libsepol/src/module_to_cil.c
+++ b/libsepol/src/module_to_cil.c
@@ -2687,7 +2687,7 @@ static int ocontext_selinux_ibpkey_to_cil(struct policydb *pdb,
 	int rc = -1;
 	struct ocontext *ibpkeycon;
 	char subnet_prefix_str[INET6_ADDRSTRLEN];
-	struct in6_addr subnet_prefix = {0};
+	struct in6_addr subnet_prefix = IN6ADDR_ANY_INIT;
 	uint16_t high;
 	uint16_t low;
 
-- 
2.14.1

[PATCH 2/6] libsepol/cil: __cil_post_db_neverallow_attr_helper() does not use extra_args

[Nicolas Iooss]

Since commit 67b410e80f09 ("libsepol/cil: Keep attributes used by
generated attributes in neverallow rules") gcc reports the following
warning when building libsepol:

    ../cil/src/cil_post.c: In function
    ‘__cil_post_db_neverallow_attr_helper’:
    ../cil/src/cil_post.c:1322:17: error: unused variable ‘db’
    [-Werror=unused-variable]
      struct cil_db *db = extra_args;
                     ^~

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
---
 libsepol/cil/src/cil_post.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/libsepol/cil/src/cil_post.c b/libsepol/cil/src/cil_post.c
index ee693d47ca5f..3e013c97e219 100644
--- a/libsepol/cil/src/cil_post.c
+++ b/libsepol/cil/src/cil_post.c
@@ -1317,10 +1317,8 @@ static void __mark_neverallow_attrs(struct cil_list *expr_list)
 	}
 }
 
-static int __cil_post_db_neverallow_attr_helper(struct cil_tree_node *node, uint32_t *finished, void *extra_args)
+static int __cil_post_db_neverallow_attr_helper(struct cil_tree_node *node, uint32_t *finished, __attribute__((unused)) void *extra_args)
 {
-	struct cil_db *db = extra_args;
-
 	switch (node->flavor) {
 	case CIL_BLOCK: {
 		struct cil_block *blk = node->data;
-- 
2.14.1

[PATCH 3/6] libsepol/cil: fix -Wwrite-strings warning

[Nicolas Iooss]

cil_defaults_to_policy() defines its third argument as non-const "char
*kind" even though it is called with literal strings. This makes gcc
report the following warning when compiling with -Wwrite-strings:

    ../cil/src/cil_policy.c: In function ‘cil_gen_policy’:
    ../cil/src/cil_policy.c:1931:60: error: passing argument 3 of
    ‘cil_defaults_to_policy’ discards ‘const’ qualifier from pointer
    target type [-Werror=discarded-qualifiers]

      cil_defaults_to_policy(out, lists[CIL_LIST_DEFAULT_USER],
                             "default_user");
                             ^~~~~~~~~~~~~~

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
---
 libsepol/cil/src/cil_policy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libsepol/cil/src/cil_policy.c b/libsepol/cil/src/cil_policy.c
index 729b6e06e9c5..6d4987c4e4e6 100644
--- a/libsepol/cil/src/cil_policy.c
+++ b/libsepol/cil/src/cil_policy.c
@@ -775,7 +775,7 @@ static void cil_classes_to_policy(FILE *out, struct cil_list *classorder)
 	}
 }
 
-static void cil_defaults_to_policy(FILE *out, struct cil_list *defaults, char *kind)
+static void cil_defaults_to_policy(FILE *out, struct cil_list *defaults, const char *kind)
 {
 	struct cil_list_item *i1, *i2, *i3;
 	struct cil_default *def;
-- 
2.14.1

[PATCH 4/6] libsepol/cil: drop wrong unused attribute

[Nicolas Iooss]

cil_gen_node() has been using its argument "db" since commit
fafe4c212bf6 ("libsepol: cil: Add ability to redeclare
types[attributes]"). Drop attribute "unused" on this argument.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
---
 libsepol/cil/src/cil_build_ast.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c
index 9fc8ab87dec2..e84336bfbe73 100644
--- a/libsepol/cil/src/cil_build_ast.c
+++ b/libsepol/cil/src/cil_build_ast.c
@@ -104,7 +104,7 @@ int cil_is_datum_multiple_decl(__attribute__((unused)) struct cil_symtab_datum *
 	return rc;
 }
 
-int cil_gen_node(__attribute__((unused)) struct cil_db *db, struct cil_tree_node *ast_node, struct cil_symtab_datum *datum, hashtab_key_t key, enum cil_sym_index sflavor, enum cil_flavor nflavor)
+int cil_gen_node(struct cil_db *db, struct cil_tree_node *ast_node, struct cil_symtab_datum *datum, hashtab_key_t key, enum cil_sym_index sflavor, enum cil_flavor nflavor)
 {
 	int rc = SEPOL_ERR;
 	symtab_t *symtab = NULL;
-- 
2.14.1

[PATCH 5/6] restorecond: check write() and daemon() results

[Nicolas Iooss]

When compiling restorecond with -Wunused, gcc 4.8.4 (from Ubuntu 14.04)
reports the following warnings:

    restorecond.c: In function ‘main’:
    restorecond.c:208:9: error: ignoring return value of ‘daemon’,
    declared with attribute warn_unused_result [-Werror=unused-result]
       daemon(0, 0);
             ^

    restorecond.c: In function ‘write_pid_file’:
    restorecond.c:106:2: error: ignoring return value of ‘write’,
    declared with attribute warn_unused_result [-Werror=unused-result]
      (void)write(pidfd, val, (unsigned int)len);
      ^

If any of these calls returns an error, it is currently silently
discarded. Add a message in order to warn about such an error.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
---
 restorecond/restorecond.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/restorecond/restorecond.c b/restorecond/restorecond.c
index f379db1e7f8d..6fbbd35dc1b3 100644
--- a/restorecond/restorecond.c
+++ b/restorecond/restorecond.c
@@ -103,7 +103,10 @@ static int write_pid_file(void)
 		pidfile = 0;
 		return 1;
 	}
-	(void)write(pidfd, val, (unsigned int)len);
+	if (write(pidfd, val, (unsigned int)len) != len) {
+		syslog(LOG_ERR, "Unable to write to pidfile (%s)", strerror(errno));
+		return 1;
+	}
 	close(pidfd);
 	return 0;
 }
@@ -204,8 +207,10 @@ int main(int argc, char **argv)
 	watch_file = server_watch_file;
 	read_config(master_fd, watch_file);
 
-	if (!debug_mode)
-		daemon(0, 0);
+	if (!debug_mode) {
+		if (daemon(0, 0) < 0)
+			exitApp("daemon");
+	}
 
 	write_pid_file();
 
-- 
2.14.1

[PATCH 6/6] Makefile: define a default value for CFLAGS

[Nicolas Iooss]

When building the project with "make DESTDIR=... install", the root
Makefile defines CFLAGS and LDFLAGS without any warning flags ("CFLAGS
+= -I$(DESTDIR)/usr/include" and "LDFLAGS += -L$(DESTDIR)/usr/lib"). As
the Makefiles in subdirectories do not override the flags with warning
flags, the code gets compiled without any enabled warning.

This leads for example to code being introduced which breaks building
libsepol from its directory, while building it from the root Makefile
still works fine.

This issue can be fixed by defining a set of flags in the root Makefile
which are used by all Makefiles in subdirectories. The flags have been
chosen following these principles:
* they are compatible with both clang and gcc,
* they already appear in at least one Makefile, and
* they are not triggered with the current git master version.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
---
 Makefile | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Makefile b/Makefile
index 7701e16252fe..6da7f7b71bdc 100644
--- a/Makefile
+++ b/Makefile
@@ -6,6 +6,16 @@ DISTCLEANSUBDIRS=libselinux libsemanage
 ifeq ($(DEBUG),1)
 	export CFLAGS = -g3 -O0 -gdwarf-2 -fno-strict-aliasing -Wall -Wshadow -Werror
 	export LDFLAGS = -g
+else
+	export CFLAGS ?= -O2 -Werror -Wall -Wextra \
+		-Wmissing-format-attribute \
+		-Wmissing-noreturn \
+		-Wpointer-arith \
+		-Wshadow \
+		-Wstrict-prototypes \
+		-Wundef \
+		-Wunused \
+		-Wwrite-strings
 endif
 
 ifneq ($(DESTDIR),)
-- 
2.14.1

Re: [PATCH 1/6] libsepol: use IN6ADDR_ANY_INIT to initialize IPv6 addresses

[jwcart2]

On 09/03/2017 08:19 AM, Nicolas Iooss wrote:
> When compiling libsepol with clang and some warning flags, the compiler
> complains about the way IPv6 addresses are initialized:
> 
>      kernel_to_cil.c:2795:35: error: suggest braces around initialization
>      of subobject [-Werror,-Wmissing-braces]
>              struct in6_addr subnet_prefix = {0};
>                                               ^
>                                               {}
> 
> When replacing the initializer as suggested, gcc 4.8.4 complains:
> 
>      kernel_to_cil.c: In function ‘write_selinux_ibpkey_rules_to_cil’:
>      kernel_to_cil.c:2795:9: error: missing initializer for field
>      ‘__in6_u’ of ‘struct in6_addr’ [-Werror=missing-field-initializers]
>        struct in6_addr subnet_prefix = {};
>               ^
> 
> Thankfully netinet/in.h provides a macro to initialize struct in6_addr
> variables:
> 
>      #define IN6ADDR_ANY_INIT { { { 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0 } } }
> 
> Both clang and gcc no longer report warnings when using this macro.
> 
> Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>

This series has been applied.

Thanks,
Jim

> ---
>   libsepol/src/kernel_to_cil.c  | 2 +-
>   libsepol/src/kernel_to_conf.c | 2 +-
>   libsepol/src/module_to_cil.c  | 2 +-
>   3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/libsepol/src/kernel_to_cil.c b/libsepol/src/kernel_to_cil.c
> index f1905a958ec0..0055c2386695 100644
> --- a/libsepol/src/kernel_to_cil.c
> +++ b/libsepol/src/kernel_to_cil.c
> @@ -2788,7 +2788,7 @@ static int write_selinux_ibpkey_rules_to_cil(FILE *out, struct policydb *pdb)
>   {
>   	struct ocontext *ibpkeycon;
>   	char subnet_prefix_str[INET6_ADDRSTRLEN];
> -	struct in6_addr subnet_prefix = {0};
> +	struct in6_addr subnet_prefix = IN6ADDR_ANY_INIT;
>   	uint16_t low;
>   	uint16_t high;
>   	char low_high_str[44]; /* 2^64 <= 20 digits so "(low high)" <= 44 chars */
> diff --git a/libsepol/src/kernel_to_conf.c b/libsepol/src/kernel_to_conf.c
> index a74873f01687..95aa92fc8c26 100644
> --- a/libsepol/src/kernel_to_conf.c
> +++ b/libsepol/src/kernel_to_conf.c
> @@ -2649,7 +2649,7 @@ static int write_selinux_ibpkey_rules_to_conf(FILE *out, struct policydb *pdb)
>   {
>   	struct ocontext *ibpkeycon;
>   	char subnet_prefix_str[INET6_ADDRSTRLEN];
> -	struct in6_addr subnet_prefix = {0};
> +	struct in6_addr subnet_prefix = IN6ADDR_ANY_INIT;
>   	uint16_t low;
>   	uint16_t high;
>   	char low_high_str[44]; /* 2^64 <= 20 digits so "low-high" <= 44 chars */
> diff --git a/libsepol/src/module_to_cil.c b/libsepol/src/module_to_cil.c
> index 619a48f8c7b6..15b58a7aacee 100644
> --- a/libsepol/src/module_to_cil.c
> +++ b/libsepol/src/module_to_cil.c
> @@ -2687,7 +2687,7 @@ static int ocontext_selinux_ibpkey_to_cil(struct policydb *pdb,
>   	int rc = -1;
>   	struct ocontext *ibpkeycon;
>   	char subnet_prefix_str[INET6_ADDRSTRLEN];
> -	struct in6_addr subnet_prefix = {0};
> +	struct in6_addr subnet_prefix = IN6ADDR_ANY_INIT;
>   	uint16_t high;
>   	uint16_t low;
>   
> 


-- 
James Carter <jwcart2@tycho.nsa.gov>
National Security Agency