[PATCH 00/11] Two bug fixes & misc code cleanup fixes

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH 00/11] Two bug fixes & misc code cleanup fixes
@ 2012-01-23 15:41 Daniel P. Berrange
  2012-01-23 15:41 ` [PATCH libselinux 01/11] Remove jump over variable declaration Daniel P. Berrange
                   ` (12 more replies)
  0 siblings, 13 replies; 14+ messages in thread
From: Daniel P. Berrange @ 2012-01-23 15:41 UTC (permalink / raw)
  To: SELinux

I was looking at the libselinux code and noticed that it did not
use much more besides '-Wall' for its default compilation flags.
As an experiment to see if there were any lurking bugs, I modified
the Makefile for libselinux to add about 30 other GCC supported
warnings.  This patch series is the result. I found 2 real bugs,
one potential use of unitialized variable in an OOM scenario,
and the other a (benign) format string mistake that meant the
user would not be told which flag was invalid.

At the same time I fixed const-correctness in several internal
and public APIs, added more printf format validation annotations,
removed old style function declarations & removed some dead
code.

One warning item that I didn't tackle here is to reduce the maximum
stack usage. With the -Wframe-larger-than flag I had to set the
libselinux stack size to 32kb, which is getting very excessive
IMHO. Most of the excessive stack usage is due to many PATH_MAX
declarations, the remainders due to a couple of large structs
placed on the stack. All of these are probably better off in
the heap long term

Hopefully the first 9 patches are fairly easily accepted. I did
not know what todo about the last 2 patches which actually add
the extra CFLAGS warnings. In most projects I would have imported
GNULIBs m4 macros for detecting support of compiler flags, but
since none of the selinux libraries use autoconf, I don't see a
good way/place to detect what compiler flags can be used.

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH libselinux 01/11] Remove jump over variable declaration
  2012-01-23 15:41 [PATCH 00/11] Two bug fixes & misc code cleanup fixes Daniel P. Berrange
@ 2012-01-23 15:41 ` Daniel P. Berrange
  2012-01-23 15:41 ` [PATCH libselinux 02/11] Ensure there is a prototype for 'matchpathcon_lib_destructor' Daniel P. Berrange
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Daniel P. Berrange @ 2012-01-23 15:41 UTC (permalink / raw)
  To: SELinux; +Cc: Daniel P. Berrange

From: "Daniel P. Berrange" <berrange@redhat.com>

seusers.c: In function ‘getseuser’:
seusers.c:273:3: error: jump skips variable initialization [-Werror=jump-misses-init]
seusers.c:317:2: note: label ‘err’ defined here
seusers.c:274:8: note: ‘fp’ declared here

* seusers.c: Declare FILE *fp at start of getseuser() method
---
 libselinux/src/seusers.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/libselinux/src/seusers.c b/libselinux/src/seusers.c
index b653cad..5cdf6c0 100644
--- a/libselinux/src/seusers.c
+++ b/libselinux/src/seusers.c
@@ -269,9 +269,10 @@ int getseuser(const char *username, const char *service,
 	size_t lineno = 0;
 	char *rec = NULL;
 	char *path=NULL;
+	FILE *fp = NULL;
 	if (asprintf(&path,"%s/logins/%s", selinux_policy_root(), username) <  0)
 		goto err;
-	FILE *fp = fopen(path, "r");
+	fp = fopen(path, "r");
 	free(path);
 	if (fp == NULL) goto err;
 	__fsetlocking(fp, FSETLOCKING_BYCALLER);
-- 
1.7.7.5


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH libselinux 02/11] Ensure there is a prototype for 'matchpathcon_lib_destructor'
  2012-01-23 15:41 [PATCH 00/11] Two bug fixes & misc code cleanup fixes Daniel P. Berrange
  2012-01-23 15:41 ` [PATCH libselinux 01/11] Remove jump over variable declaration Daniel P. Berrange
@ 2012-01-23 15:41 ` Daniel P. Berrange
  2012-01-23 15:41 ` [PATCH libselinux 03/11] Fix old style function definitions Daniel P. Berrange
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Daniel P. Berrange @ 2012-01-23 15:41 UTC (permalink / raw)
  To: SELinux; +Cc: Daniel P. Berrange

From: "Daniel P. Berrange" <berrange@redhat.com>

---
 libselinux/src/matchpathcon.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/libselinux/src/matchpathcon.c b/libselinux/src/matchpathcon.c
index 48f7a11..804442b 100644
--- a/libselinux/src/matchpathcon.c
+++ b/libselinux/src/matchpathcon.c
@@ -292,6 +292,8 @@ static void matchpathcon_thread_destructor(void __attribute__((unused)) *ptr)
 	matchpathcon_fini();
 }
 
+void __attribute__((destructor)) matchpathcon_lib_destructor(void);
+
 void __attribute__((destructor)) matchpathcon_lib_destructor(void)
 {
 	if (destructor_key_initialized)
-- 
1.7.7.5


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH libselinux 03/11] Fix old style function definitions
  2012-01-23 15:41 [PATCH 00/11] Two bug fixes & misc code cleanup fixes Daniel P. Berrange
  2012-01-23 15:41 ` [PATCH libselinux 01/11] Remove jump over variable declaration Daniel P. Berrange
  2012-01-23 15:41 ` [PATCH libselinux 02/11] Ensure there is a prototype for 'matchpathcon_lib_destructor' Daniel P. Berrange
@ 2012-01-23 15:41 ` Daniel P. Berrange
  2012-01-23 15:41 ` [PATCH libselinux 04/11] Fix const-correctness Daniel P. Berrange
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Daniel P. Berrange @ 2012-01-23 15:41 UTC (permalink / raw)
  To: SELinux; +Cc: Daniel P. Berrange

From: "Daniel P. Berrange" <berrange@redhat.com>

Add 'void' parameter to all functions which take no arguments

* selinux_config.c: s/()/(void)/
---
 libselinux/src/selinux_config.c |   52 +++++++++++++++++++-------------------
 1 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/libselinux/src/selinux_config.c b/libselinux/src/selinux_config.c
index f4c33df..f42cb7c 100644
--- a/libselinux/src/selinux_config.c
+++ b/libselinux/src/selinux_config.c
@@ -246,172 +246,172 @@ static const char *get_path(int idx)
 	return file_paths[idx];
 }
 
-const char *selinux_default_type_path()
+const char *selinux_default_type_path(void)
 {
 	return get_path(DEFAULT_TYPE);
 }
 
 hidden_def(selinux_default_type_path)
 
-const char *selinux_policy_root()
+const char *selinux_policy_root(void)
 {
 	__selinux_once(once, init_selinux_config);
 	return selinux_policyroot;
 }
 
-const char *selinux_path()
+const char *selinux_path(void)
 {
 	return selinux_rootpath;
 }
 
 hidden_def(selinux_path)
 
-const char *selinux_default_context_path()
+const char *selinux_default_context_path(void)
 {
 	return get_path(DEFAULT_CONTEXTS);
 }
 
 hidden_def(selinux_default_context_path)
 
-const char *selinux_securetty_types_path()
+const char *selinux_securetty_types_path(void)
 {
 	return get_path(SECURETTY_TYPES);
 }
 
 hidden_def(selinux_securetty_types_path)
 
-const char *selinux_failsafe_context_path()
+const char *selinux_failsafe_context_path(void)
 {
 	return get_path(FAILSAFE_CONTEXT);
 }
 
 hidden_def(selinux_failsafe_context_path)
 
-const char *selinux_removable_context_path()
+const char *selinux_removable_context_path(void)
 {
 	return get_path(REMOVABLE_CONTEXT);
 }
 
 hidden_def(selinux_removable_context_path)
 
-const char *selinux_binary_policy_path()
+const char *selinux_binary_policy_path(void)
 {
 	return get_path(BINPOLICY);
 }
 
 hidden_def(selinux_binary_policy_path)
 
-const char *selinux_file_context_path()
+const char *selinux_file_context_path(void)
 {
 	return get_path(FILE_CONTEXTS);
 }
 
 hidden_def(selinux_file_context_path)
 
-const char *selinux_homedir_context_path()
+const char *selinux_homedir_context_path(void)
 {
 	return get_path(HOMEDIR_CONTEXTS);
 }
 
 hidden_def(selinux_homedir_context_path)
 
-const char *selinux_media_context_path()
+const char *selinux_media_context_path(void)
 {
 	return get_path(MEDIA_CONTEXTS);
 }
 
 hidden_def(selinux_media_context_path)
 
-const char *selinux_customizable_types_path()
+const char *selinux_customizable_types_path(void)
 {
 	return get_path(CUSTOMIZABLE_TYPES);
 }
 
 hidden_def(selinux_customizable_types_path)
 
-const char *selinux_contexts_path()
+const char *selinux_contexts_path(void)
 {
 	return get_path(CONTEXTS_DIR);
 }
 
-const char *selinux_user_contexts_path()
+const char *selinux_user_contexts_path(void)
 {
 	return get_path(USER_CONTEXTS);
 }
 
 hidden_def(selinux_user_contexts_path)
 
-const char *selinux_booleans_path()
+const char *selinux_booleans_path(void)
 {
 	return get_path(BOOLEANS);
 }
 
 hidden_def(selinux_booleans_path)
 
-const char *selinux_users_path()
+const char *selinux_users_path(void)
 {
 	return get_path(USERS_DIR);
 }
 
 hidden_def(selinux_users_path)
 
-const char *selinux_usersconf_path()
+const char *selinux_usersconf_path(void)
 {
 	return get_path(SEUSERS);
 }
 
 hidden_def(selinux_usersconf_path)
 
-const char *selinux_translations_path()
+const char *selinux_translations_path(void)
 {
 	return get_path(TRANSLATIONS);
 }
 
 hidden_def(selinux_translations_path)
 
-const char *selinux_colors_path()
+const char *selinux_colors_path(void)
 {
 	return get_path(COLORS);
 }
 
 hidden_def(selinux_colors_path)
 
-const char *selinux_netfilter_context_path()
+const char *selinux_netfilter_context_path(void)
 {
 	return get_path(NETFILTER_CONTEXTS);
 }
 
 hidden_def(selinux_netfilter_context_path)
 
-const char *selinux_file_context_homedir_path()
+const char *selinux_file_context_homedir_path(void)
 {
 	return get_path(FILE_CONTEXTS_HOMEDIR);
 }
 
 hidden_def(selinux_file_context_homedir_path)
 
-const char *selinux_file_context_local_path()
+const char *selinux_file_context_local_path(void)
 {
 	return get_path(FILE_CONTEXTS_LOCAL);
 }
 
 hidden_def(selinux_file_context_local_path)
 
-const char *selinux_x_context_path()
+const char *selinux_x_context_path(void)
 {
 	return get_path(X_CONTEXTS);
 }
 
 hidden_def(selinux_x_context_path)
 
-const char *selinux_virtual_domain_context_path()
+const char *selinux_virtual_domain_context_path(void)
 {
 	return get_path(VIRTUAL_DOMAIN);
 }
 
 hidden_def(selinux_virtual_domain_context_path)
 
-const char *selinux_virtual_image_context_path()
+const char *selinux_virtual_image_context_path(void)
 {
 	return get_path(VIRTUAL_IMAGE);
 }
@@ -430,7 +430,7 @@ const char * selinux_file_context_subs_dist_path(void) {
 
 hidden_def(selinux_file_context_subs_dist_path)
 
-const char *selinux_sepgsql_context_path()
+const char *selinux_sepgsql_context_path(void)
 {
 	return get_path(SEPGSQL_CONTEXTS);
 }
-- 
1.7.7.5


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH libselinux 04/11] Fix const-correctness
  2012-01-23 15:41 [PATCH 00/11] Two bug fixes & misc code cleanup fixes Daniel P. Berrange
                   ` (2 preceding siblings ...)
  2012-01-23 15:41 ` [PATCH libselinux 03/11] Fix old style function definitions Daniel P. Berrange
@ 2012-01-23 15:41 ` Daniel P. Berrange
  2012-01-23 15:41 ` [PATCH libselinux 05/11] Remove unused flush_class_cache method Daniel P. Berrange
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Daniel P. Berrange @ 2012-01-23 15:41 UTC (permalink / raw)
  To: SELinux; +Cc: Daniel P. Berrange

From: "Daniel P. Berrange" <berrange@redhat.com>

* include/selinux/selinux.h, src/init.c: set_selinuxmnt should take
  a const char *mntpath
* src/get_default_type.c: Avoid bad cast discarding const
* load_policy.c: Fix var decl to avoid discarding const
---
 libselinux/include/selinux/selinux.h |    2 +-
 libselinux/src/get_default_type.c    |    3 ++-
 libselinux/src/init.c                |    4 ++--
 libselinux/src/load_policy.c         |    2 +-
 4 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/libselinux/include/selinux/selinux.h b/libselinux/include/selinux/selinux.h
index 2985f6f..e782b14 100644
--- a/libselinux/include/selinux/selinux.h
+++ b/libselinux/include/selinux/selinux.h
@@ -537,7 +537,7 @@ extern int selinux_check_securetty_context(const security_context_t tty_context)
    Normally, this is determined automatically during libselinux 
    initialization, but this is not always possible, e.g. for /sbin/init
    which performs the initial mount of selinuxfs. */
-void set_selinuxmnt(char *mnt);
+void set_selinuxmnt(const char *mnt);
 
 /* Check if selinuxfs exists as a kernel filesystem */
 int selinuxfs_exists(void);
diff --git a/libselinux/src/get_default_type.c b/libselinux/src/get_default_type.c
index ca3d291..27f2ae5 100644
--- a/libselinux/src/get_default_type.c
+++ b/libselinux/src/get_default_type.c
@@ -27,7 +27,8 @@ int get_default_type(const char *role, char **type)
 static int find_default_type(FILE * fp, const char *role, char **type)
 {
 	char buf[250];
-	char *ptr = "", *end, *t;
+	const char *ptr = "", *end;
+	char *t;
 	size_t len;
 	int found = 0;
 
diff --git a/libselinux/src/init.c b/libselinux/src/init.c
index 00afde7..6d1ef33 100644
--- a/libselinux/src/init.c
+++ b/libselinux/src/init.c
@@ -28,7 +28,7 @@ int obj_class_compat = 1;
    * The file system is read/write
    * then set this as the default file system.
 */
-static int verify_selinuxmnt(char *mnt)
+static int verify_selinuxmnt(const char *mnt)
 {
 	struct statfs sfbuf;
 	int rc;
@@ -139,7 +139,7 @@ void fini_selinuxmnt(void)
 
 hidden_def(fini_selinuxmnt)
 
-void set_selinuxmnt(char *mnt)
+void set_selinuxmnt(const char *mnt)
 {
 	selinux_mnt = strdup(mnt);
 }
diff --git a/libselinux/src/load_policy.c b/libselinux/src/load_policy.c
index f569664..10e29b9 100644
--- a/libselinux/src/load_policy.c
+++ b/libselinux/src/load_policy.c
@@ -369,7 +369,7 @@ int selinux_init_load_policy(int *enforce)
 	 * Check for the existence of SELinux via selinuxfs, and 
 	 * mount it if present for use in the calls below.  
 	 */
-	char *mntpoint = NULL;
+	const char *mntpoint = NULL;
 	if (mount(SELINUXFS, SELINUXMNT, SELINUXFS, 0, 0) == 0 || errno == EBUSY) {
 		mntpoint = SELINUXMNT;
 	} else {
-- 
1.7.7.5


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH libselinux 05/11] Remove unused flush_class_cache method
  2012-01-23 15:41 [PATCH 00/11] Two bug fixes & misc code cleanup fixes Daniel P. Berrange
                   ` (3 preceding siblings ...)
  2012-01-23 15:41 ` [PATCH libselinux 04/11] Fix const-correctness Daniel P. Berrange
@ 2012-01-23 15:41 ` Daniel P. Berrange
  2012-01-23 15:41 ` [PATCH libselinux 06/11] Add prototype decl for destructor Daniel P. Berrange
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Daniel P. Berrange @ 2012-01-23 15:41 UTC (permalink / raw)
  To: SELinux; +Cc: Daniel P. Berrange

From: "Daniel P. Berrange" <berrange@redhat.com>

* stringrep.c: Delete flush_class_cache
---
 libselinux/src/stringrep.c |   22 ----------------------
 1 files changed, 0 insertions(+), 22 deletions(-)

diff --git a/libselinux/src/stringrep.c b/libselinux/src/stringrep.c
index f0167e7..176ac34 100644
--- a/libselinux/src/stringrep.c
+++ b/libselinux/src/stringrep.c
@@ -305,28 +305,6 @@ err1:
 	return NULL;
 }
 
-void flush_class_cache(void)
-{
-	struct discover_class_node *cur = discover_class_cache, *prev = NULL;
-	size_t i;
-
-	while (cur != NULL) {
-		free(cur->name);
-
-		for (i=0 ; i<MAXVECTORS ; i++)
-			free(cur->perms[i]);
-
-		free(cur->perms);
-
-		prev = cur;
-		cur = cur->next;
-
-		free(prev);
-	}
-
-	discover_class_cache = NULL;
-}
-
 static security_class_t string_to_security_class_compat(const char *s)
 {
 	unsigned int val;
-- 
1.7.7.5


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH libselinux 06/11] Add prototype decl for destructor
  2012-01-23 15:41 [PATCH 00/11] Two bug fixes & misc code cleanup fixes Daniel P. Berrange
                   ` (4 preceding siblings ...)
  2012-01-23 15:41 ` [PATCH libselinux 05/11] Remove unused flush_class_cache method Daniel P. Berrange
@ 2012-01-23 15:41 ` Daniel P. Berrange
  2012-01-23 15:41 ` [PATCH libselinux 07/11] Add more printf format annotations Daniel P. Berrange
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Daniel P. Berrange @ 2012-01-23 15:41 UTC (permalink / raw)
  To: SELinux; +Cc: Daniel P. Berrange

From: "Daniel P. Berrange" <berrange@redhat.com>

---
 libselinux/src/setrans_client.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/libselinux/src/setrans_client.c b/libselinux/src/setrans_client.c
index e074142..9432f49 100644
--- a/libselinux/src/setrans_client.c
+++ b/libselinux/src/setrans_client.c
@@ -253,6 +253,8 @@ static void setrans_thread_destructor(void __attribute__((unused)) *unused)
 	free(prev_r2c_raw);
 }
 
+void __attribute__((destructor)) setrans_lib_destructor(void);
+
 void __attribute__((destructor)) setrans_lib_destructor(void)
 {
 	if (destructor_key_initialized)
-- 
1.7.7.5


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH libselinux 07/11] Add more printf format annotations
  2012-01-23 15:41 [PATCH 00/11] Two bug fixes & misc code cleanup fixes Daniel P. Berrange
                   ` (5 preceding siblings ...)
  2012-01-23 15:41 ` [PATCH libselinux 06/11] Add prototype decl for destructor Daniel P. Berrange
@ 2012-01-23 15:41 ` Daniel P. Berrange
  2012-01-23 15:41 ` [PATCH libselinux 08/11] Add printf format attribute annotation to die() method Daniel P. Berrange
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Daniel P. Berrange @ 2012-01-23 15:41 UTC (permalink / raw)
  To: SELinux; +Cc: Daniel P. Berrange

From: "Daniel P. Berrange" <berrange@redhat.com>

The public avc.h file must use a printf annotation in the struct
callback members, otherwise application code will get compiler
warnings that the method should have an annotation set.
---
 libselinux/include/selinux/avc.h |    2 +-
 libselinux/src/avc_internal.h    |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libselinux/include/selinux/avc.h b/libselinux/include/selinux/avc.h
index da18e41..9655957 100644
--- a/libselinux/include/selinux/avc.h
+++ b/libselinux/include/selinux/avc.h
@@ -130,7 +130,7 @@ struct avc_memory_callback {
 
 struct avc_log_callback {
 	/* log the printf-style format and arguments. */
-	void (*func_log) (const char *fmt, ...);
+	void (*func_log) (const char *fmt, ...) __attribute__((__format__(printf, 1, 2)));
 	/* store a string representation of auditdata (corresponding
 	   to the given security class) into msgbuf. */
 	void (*func_audit) (void *auditdata, security_class_t cls,
diff --git a/libselinux/src/avc_internal.h b/libselinux/src/avc_internal.h
index 53610e8..f851659 100644
--- a/libselinux/src/avc_internal.h
+++ b/libselinux/src/avc_internal.h
@@ -20,7 +20,7 @@
 extern void *(*avc_func_malloc) (size_t) hidden;
 extern void (*avc_func_free) (void *)hidden;
 
-extern void (*avc_func_log) (const char *, ...)hidden;
+extern void (*avc_func_log) (const char *, ...) __attribute__((__format__(printf,1,2))) hidden;
 extern void (*avc_func_audit) (void *, security_class_t, char *, size_t)hidden;
 
 extern int avc_using_threads hidden;
-- 
1.7.7.5


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH libselinux 08/11] Add printf format attribute annotation to die() method
  2012-01-23 15:41 [PATCH 00/11] Two bug fixes & misc code cleanup fixes Daniel P. Berrange
                   ` (6 preceding siblings ...)
  2012-01-23 15:41 ` [PATCH libselinux 07/11] Add more printf format annotations Daniel P. Berrange
@ 2012-01-23 15:41 ` Daniel P. Berrange
  2012-01-23 15:41 ` [PATCH libselinux 09/11] Fix const-ness of parameters & make usage() methods static Daniel P. Berrange
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Daniel P. Berrange @ 2012-01-23 15:41 UTC (permalink / raw)
  To: SELinux; +Cc: Daniel P. Berrange

From: "Daniel P. Berrange" <berrange@redhat.com>

Annotating the die method as taking printf format exposes
a bug in error reporting
---
 libselinux/utils/avcstat.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libselinux/utils/avcstat.c b/libselinux/utils/avcstat.c
index 772118a..7239ef2 100644
--- a/libselinux/utils/avcstat.c
+++ b/libselinux/utils/avcstat.c
@@ -43,7 +43,7 @@ static char buf[DEF_BUF_SIZE];
 /* selinuxfs mount point */
 extern char *selinux_mnt;
 
-static void die(const char *msg, ...)
+static __attribute__((__format__(printf,1,2))) void die(const char *msg, ...)
 {
 	va_list args;
 
@@ -118,7 +118,7 @@ int main(int argc, char **argv)
 			exit(0);
 		default:
 			usage();
-			die("unrecognized parameter", i);
+			die("unrecognized parameter '%c'", i);
 		}
 	}
 
-- 
1.7.7.5


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH libselinux 09/11] Fix const-ness of parameters & make usage() methods static
  2012-01-23 15:41 [PATCH 00/11] Two bug fixes & misc code cleanup fixes Daniel P. Berrange
                   ` (7 preceding siblings ...)
  2012-01-23 15:41 ` [PATCH libselinux 08/11] Add printf format attribute annotation to die() method Daniel P. Berrange
@ 2012-01-23 15:41 ` Daniel P. Berrange
  2012-01-23 15:41 ` [PATCH libselinux 10/11] Enable many more gcc warnings for libselinux/src/ builds Daniel P. Berrange
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Daniel P. Berrange @ 2012-01-23 15:41 UTC (permalink / raw)
  To: SELinux; +Cc: Daniel P. Berrange

From: "Daniel P. Berrange" <berrange@redhat.com>

---
 libselinux/utils/getconlist.c                      |    2 +-
 libselinux/utils/getdefaultcon.c                   |    2 +-
 libselinux/utils/getsebool.c                       |    2 +-
 libselinux/utils/matchpathcon.c                    |    4 ++--
 libselinux/utils/selinux_check_securetty_context.c |    2 +-
 libselinux/utils/selinuxexeccon.c                  |    2 +-
 libselinux/utils/setenforce.c                      |    2 +-
 libselinux/utils/togglesebool.c                    |    2 +-
 8 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/libselinux/utils/getconlist.c b/libselinux/utils/getconlist.c
index 4f473e4..94c9bff 100644
--- a/libselinux/utils/getconlist.c
+++ b/libselinux/utils/getconlist.c
@@ -9,7 +9,7 @@
 #include <selinux/selinux.h>
 #include <selinux/get_context_list.h>
 
-void usage(char *name, char *detail, int rc)
+static void usage(const char *name, const char *detail, int rc)
 {
 	fprintf(stderr, "usage:  %s [-l level] user [context]\n", name);
 	if (detail)
diff --git a/libselinux/utils/getdefaultcon.c b/libselinux/utils/getdefaultcon.c
index e6eb98b..049e75c 100644
--- a/libselinux/utils/getdefaultcon.c
+++ b/libselinux/utils/getdefaultcon.c
@@ -9,7 +9,7 @@
 #include <selinux/selinux.h>
 #include <selinux/get_context_list.h>
 
-void usage(char *name, char *detail, int rc)
+static void usage(const char *name, const char *detail, int rc)
 {
 	fprintf(stderr, "usage:  %s [-l level] user fromcon\n", name);
 	if (detail)
diff --git a/libselinux/utils/getsebool.c b/libselinux/utils/getsebool.c
index cab2bb9..3a90449 100644
--- a/libselinux/utils/getsebool.c
+++ b/libselinux/utils/getsebool.c
@@ -6,7 +6,7 @@
 #include <string.h>
 #include <selinux/selinux.h>
 
-void usage(const char *progname)
+static void usage(const char *progname)
 {
 	fprintf(stderr, "usage:  %s -a or %s boolean...\n", progname, progname);
 	exit(1);
diff --git a/libselinux/utils/matchpathcon.c b/libselinux/utils/matchpathcon.c
index 5f0a4c2..b1adadd 100644
--- a/libselinux/utils/matchpathcon.c
+++ b/libselinux/utils/matchpathcon.c
@@ -13,7 +13,7 @@
 #include <stdlib.h>
 
 
-void usage(const char *progname)
+static void usage(const char *progname)
 {
 	fprintf(stderr,
 		"usage:  %s [-N] [-n] [-f file_contexts] [-p prefix] [-Vq] path...\n",
@@ -21,7 +21,7 @@ void usage(const char *progname)
 	exit(1);
 }
 
-int printmatchpathcon(char *path, int header, int mode)
+static int printmatchpathcon(const char *path, int header, int mode)
 {
 	char *buf;
 	int rc = matchpathcon(path, mode, &buf);
diff --git a/libselinux/utils/selinux_check_securetty_context.c b/libselinux/utils/selinux_check_securetty_context.c
index 95bfb7f..b158eb3 100644
--- a/libselinux/utils/selinux_check_securetty_context.c
+++ b/libselinux/utils/selinux_check_securetty_context.c
@@ -9,7 +9,7 @@
 #include <sys/errno.h>
 #include <selinux/selinux.h>
 
-void usage(const char *progname)
+static void usage(const char *progname)
 {
 	fprintf(stderr, "usage:  %s tty_context...\n", progname);
 	exit(1);
diff --git a/libselinux/utils/selinuxexeccon.c b/libselinux/utils/selinuxexeccon.c
index c55fde9..476f564 100644
--- a/libselinux/utils/selinuxexeccon.c
+++ b/libselinux/utils/selinuxexeccon.c
@@ -9,7 +9,7 @@
 #include <selinux/flask.h>
 #include <selinux/selinux.h>
 
-void usage(char *name, char *detail, int rc)
+static void usage(const char *name, const char *detail, int rc)
 {
 	fprintf(stderr, "usage:  %s command [ fromcon ]\n", name);
 	if (detail)
diff --git a/libselinux/utils/setenforce.c b/libselinux/utils/setenforce.c
index e45b804..df58597 100644
--- a/libselinux/utils/setenforce.c
+++ b/libselinux/utils/setenforce.c
@@ -6,7 +6,7 @@
 #include <strings.h>
 #include <selinux/selinux.h>
 
-void usage(const char *progname)
+static void usage(const char *progname)
 {
 	fprintf(stderr, "usage:  %s [ Enforcing | Permissive | 1 | 0 ]\n",
 		progname);
diff --git a/libselinux/utils/togglesebool.c b/libselinux/utils/togglesebool.c
index 680ed8d..ad0d2a2 100644
--- a/libselinux/utils/togglesebool.c
+++ b/libselinux/utils/togglesebool.c
@@ -10,7 +10,7 @@
 
 /* Attempt to rollback the transaction. No need to check error
    codes since this is rolling back something that blew up. */
-void rollback(int argc, char **argv)
+static void rollback(int argc, char **argv)
 {
 	int i;
 
-- 
1.7.7.5


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH libselinux 10/11] Enable many more gcc warnings for libselinux/src/ builds
  2012-01-23 15:41 [PATCH 00/11] Two bug fixes & misc code cleanup fixes Daniel P. Berrange
                   ` (8 preceding siblings ...)
  2012-01-23 15:41 ` [PATCH libselinux 09/11] Fix const-ness of parameters & make usage() methods static Daniel P. Berrange
@ 2012-01-23 15:41 ` Daniel P. Berrange
  2012-01-23 15:41 ` [PATCH libselinux 11/11] Enable many more gcc warnings for libselinux/utils builds Daniel P. Berrange
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Daniel P. Berrange @ 2012-01-23 15:41 UTC (permalink / raw)
  To: SELinux; +Cc: Daniel P. Berrange

From: "Daniel P. Berrange" <berrange@redhat.com>

XXX:  -Wno-redundant-decls really shouldn't be set, if some way
can be found to deal with warnings generated by dso.h

XXX: the maximum stack size should be much lower, but there
are too many functions using PATH_MAX which need to be rewritten
to use the heap instead.

XXX: probe for whether the user's GCC supports a flag ?
---
 libselinux/src/Makefile |   22 +++++++++++++++++++++-
 1 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile
index 1ddddb0..985842d 100644
--- a/libselinux/src/Makefile
+++ b/libselinux/src/Makefile
@@ -51,9 +51,29 @@ endif
 GENERATED=$(SWIGCOUT) $(SWIGRUBYCOUT) selinuxswig_python_exception.i
 SRCS= $(filter-out $(UNUSED_SRCS) $(GENERATED) audit2why.c, $(wildcard *.c))
 
+MAX_STACK_SIZE=32768
+
 OBJS= $(patsubst %.c,%.o,$(SRCS))
 LOBJS= $(patsubst %.c,%.lo,$(SRCS))
-CFLAGS ?= -Werror -Wall -W -Wundef -Wshadow -Wmissing-noreturn -Wmissing-format-attribute
+CFLAGS ?= -Wall -W -Wundef -Wformat-y2k -Wformat-security -Winit-self -Wmissing-include-dirs \
+          -Wunused -Wunknown-pragmas -Wstrict-aliasing -Wshadow -Wpointer-arith \
+          -Wbad-function-cast -Wcast-align -Wwrite-strings -Wlogical-op -Waggregate-return \
+          -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes \
+          -Wmissing-declarations -Wmissing-noreturn -Wmissing-format-attribute \
+          -Wredundant-decls -Wnested-externs -Winline -Winvalid-pch -Wvolatile-register-var \
+          -Wdisabled-optimization -Wbuiltin-macro-redefined -Wmudflap -Wpacked-bitfield-compat \
+          -Wsync-nand -Wattributes -Wcoverage-mismatch -Wmultichar -Wcpp \
+          -Wdeprecated-declarations -Wdiv-by-zero -Wdouble-promotion -Wendif-labels -Wextra \
+          -Wformat-contains-nul -Wformat-extra-args -Wformat-zero-length -Wformat=2 -Wmultichar \
+          -Wnormalized=nfc -Woverflow -Wpointer-to-int-cast -Wpragmas -Wsuggest-attribute=const \
+          -Wsuggest-attribute=noreturn -Wsuggest-attribute=pure -Wtrampolines \
+          -Wno-missing-field-initializers -Wno-sign-compare -Wjump-misses-init \
+          -Wno-format-nonliteral -Wframe-larger-than=$(MAX_STACK_SIZE) -Wp,-D_FORTIFY_SOURCE=2 \
+          -fstack-protector-all --param=ssp-buffer-size=4 -fexceptions \
+          -fasynchronous-unwind-tables -fdiagnostics-show-option -funit-at-a-time \
+          -fipa-pure-const -Wno-suggest-attribute=pure -Wno-suggest-attribute=const \
+          -Werror -Wno-aggregate-return -Wno-redundant-decls
+
 override CFLAGS += -I../include -I$(INCLUDEDIR) -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 $(EMFLAGS)
 RANLIB=ranlib
 
-- 
1.7.7.5


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH libselinux 11/11] Enable many more gcc warnings for libselinux/utils builds
  2012-01-23 15:41 [PATCH 00/11] Two bug fixes & misc code cleanup fixes Daniel P. Berrange
                   ` (9 preceding siblings ...)
  2012-01-23 15:41 ` [PATCH libselinux 10/11] Enable many more gcc warnings for libselinux/src/ builds Daniel P. Berrange
@ 2012-01-23 15:41 ` Daniel P. Berrange
  2012-01-23 16:30 ` [PATCH 00/11] Two bug fixes & misc code cleanup fixes Daniel J Walsh
  2012-01-23 18:13 ` [PATCH 12/11] Change annotation on include/selinux/avc.h to avoid upsetting SWIG Daniel P. Berrange
  12 siblings, 0 replies; 14+ messages in thread
From: Daniel P. Berrange @ 2012-01-23 15:41 UTC (permalink / raw)
  To: SELinux; +Cc: Daniel P. Berrange

From: "Daniel P. Berrange" <berrange@redhat.com>

---
 libselinux/utils/Makefile |   20 +++++++++++++++++++-
 1 files changed, 19 insertions(+), 1 deletions(-)

diff --git a/libselinux/utils/Makefile b/libselinux/utils/Makefile
index 6f5aa52..4b2f348 100644
--- a/libselinux/utils/Makefile
+++ b/libselinux/utils/Makefile
@@ -4,7 +4,25 @@ LIBDIR ?= $(PREFIX)/lib
 BINDIR ?= $(PREFIX)/sbin
 _BINDIR ?= $(DESTDIR)/sbin
 
-CFLAGS ?= -Werror -Wall -W
+MAX_STACK_SIZE=8192
+CFLAGS ?= -Wall -W -Wundef -Wformat-y2k -Wformat-security -Winit-self -Wmissing-include-dirs \
+          -Wunused -Wunknown-pragmas -Wstrict-aliasing -Wshadow -Wpointer-arith \
+          -Wbad-function-cast -Wcast-align -Wwrite-strings -Wlogical-op -Waggregate-return \
+          -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes \
+          -Wmissing-declarations -Wmissing-noreturn -Wmissing-format-attribute \
+          -Wredundant-decls -Wnested-externs -Winline -Winvalid-pch -Wvolatile-register-var \
+          -Wdisabled-optimization -Wbuiltin-macro-redefined -Wmudflap -Wpacked-bitfield-compat \
+          -Wsync-nand -Wattributes -Wcoverage-mismatch -Wmultichar -Wcpp \
+          -Wdeprecated-declarations -Wdiv-by-zero -Wdouble-promotion -Wendif-labels -Wextra \
+          -Wformat-contains-nul -Wformat-extra-args -Wformat-zero-length -Wformat=2 -Wmultichar \
+          -Wnormalized=nfc -Woverflow -Wpointer-to-int-cast -Wpragmas -Wsuggest-attribute=const \
+          -Wsuggest-attribute=noreturn -Wsuggest-attribute=pure -Wtrampolines \
+          -Wno-missing-field-initializers -Wno-sign-compare -Wjump-misses-init \
+          -Wno-format-nonliteral -Wframe-larger-than=$(MAX_STACK_SIZE) -Wp,-D_FORTIFY_SOURCE=2 \
+          -fstack-protector-all --param=ssp-buffer-size=4 -fexceptions \
+          -fasynchronous-unwind-tables -fdiagnostics-show-option -funit-at-a-time \
+          -fipa-pure-const -Wno-suggest-attribute=pure -Wno-suggest-attribute=const \
+          -Werror -Wno-aggregate-return -Wno-redundant-decls
 override CFLAGS += -I../include -D_GNU_SOURCE $(EMFLAGS)
 LDLIBS += -L../src -lselinux -L$(LIBDIR)
 
-- 
1.7.7.5


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 00/11] Two bug fixes & misc code cleanup fixes
  2012-01-23 15:41 [PATCH 00/11] Two bug fixes & misc code cleanup fixes Daniel P. Berrange
                   ` (10 preceding siblings ...)
  2012-01-23 15:41 ` [PATCH libselinux 11/11] Enable many more gcc warnings for libselinux/utils builds Daniel P. Berrange
@ 2012-01-23 16:30 ` Daniel J Walsh
  2012-01-23 18:13 ` [PATCH 12/11] Change annotation on include/selinux/avc.h to avoid upsetting SWIG Daniel P. Berrange
  12 siblings, 0 replies; 14+ messages in thread
From: Daniel J Walsh @ 2012-01-23 16:30 UTC (permalink / raw)
  To: Daniel P. Berrange; +Cc: SELinux

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 01/23/2012 10:41 AM, Daniel P. Berrange wrote:
> I was looking at the libselinux code and noticed that it did not 
> use much more besides '-Wall' for its default compilation flags. As
> an experiment to see if there were any lurking bugs, I modified the
> Makefile for libselinux to add about 30 other GCC supported 
> warnings.  This patch series is the result. I found 2 real bugs, 
> one potential use of unitialized variable in an OOM scenario, and
> the other a (benign) format string mistake that meant the user
> would not be told which flag was invalid.
> 
> At the same time I fixed const-correctness in several internal and
> public APIs, added more printf format validation annotations, 
> removed old style function declarations & removed some dead code.
> 
> One warning item that I didn't tackle here is to reduce the
> maximum stack usage. With the -Wframe-larger-than flag I had to set
> the libselinux stack size to 32kb, which is getting very excessive 
> IMHO. Most of the excessive stack usage is due to many PATH_MAX 
> declarations, the remainders due to a couple of large structs 
> placed on the stack. All of these are probably better off in the
> heap long term
> 
> Hopefully the first 9 patches are fairly easily accepted. I did not
> know what todo about the last 2 patches which actually add the
> extra CFLAGS warnings. In most projects I would have imported 
> GNULIBs m4 macros for detecting support of compiler flags, but 
> since none of the selinux libraries use autoconf, I don't see a 
> good way/place to detect what compiler flags can be used.
> 
> 
> -- This message was distributed to subscribers of the selinux
> mailing list. If you no longer wish to subscribe, send mail to
> majordomo@tycho.nsa.gov with the words "unsubscribe selinux"
> without quotes as the message.
> 
> 

I have added all of your patches to the libselinux-2.1.9-3.fc17
in Fedora/Rawhide.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk8dizkACgkQrlYvE4MpobPxlQCfefRzBAjs8+R7DeIO2/CiJmLA
RKAAnjFmjKjEj8CUEO2rTO0Ir3GvyIA/
=/68v
-----END PGP SIGNATURE-----

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 12/11] Change annotation on include/selinux/avc.h to avoid upsetting SWIG
  2012-01-23 15:41 [PATCH 00/11] Two bug fixes & misc code cleanup fixes Daniel P. Berrange
                   ` (11 preceding siblings ...)
  2012-01-23 16:30 ` [PATCH 00/11] Two bug fixes & misc code cleanup fixes Daniel J Walsh
@ 2012-01-23 18:13 ` Daniel P. Berrange
  12 siblings, 0 replies; 14+ messages in thread
From: Daniel P. Berrange @ 2012-01-23 18:13 UTC (permalink / raw)
  To: SELinux; +Cc: Daniel P. Berrange

From: "Daniel P. Berrange" <berrange@redhat.com>

The earlier patch to avc.c put the struct member annotation at
the end of the line, which works fine for GCC, but upsets SWIG.
Equivalent code in selinux.h demonstrates how to place the
annotation without upsetting SWIG.
---
 libselinux/include/selinux/avc.h |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/libselinux/include/selinux/avc.h b/libselinux/include/selinux/avc.h
index 9655957..87a2b12 100644
--- a/libselinux/include/selinux/avc.h
+++ b/libselinux/include/selinux/avc.h
@@ -130,7 +130,11 @@ struct avc_memory_callback {
 
 struct avc_log_callback {
 	/* log the printf-style format and arguments. */
-	void (*func_log) (const char *fmt, ...) __attribute__((__format__(printf, 1, 2)));
+	void
+#ifdef __GNUC__
+__attribute__ ((format(printf, 1, 2)))
+#endif
+	(*func_log) (const char *fmt, ...);
 	/* store a string representation of auditdata (corresponding
 	   to the given security class) into msgbuf. */
 	void (*func_audit) (void *auditdata, security_class_t cls,
-- 
1.7.7.5


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

^ permalink raw reply related	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2012-01-23 18:13 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-01-23 15:41 [PATCH 00/11] Two bug fixes & misc code cleanup fixes Daniel P. Berrange
2012-01-23 15:41 ` [PATCH libselinux 01/11] Remove jump over variable declaration Daniel P. Berrange
2012-01-23 15:41 ` [PATCH libselinux 02/11] Ensure there is a prototype for 'matchpathcon_lib_destructor' Daniel P. Berrange
2012-01-23 15:41 ` [PATCH libselinux 03/11] Fix old style function definitions Daniel P. Berrange
2012-01-23 15:41 ` [PATCH libselinux 04/11] Fix const-correctness Daniel P. Berrange
2012-01-23 15:41 ` [PATCH libselinux 05/11] Remove unused flush_class_cache method Daniel P. Berrange
2012-01-23 15:41 ` [PATCH libselinux 06/11] Add prototype decl for destructor Daniel P. Berrange
2012-01-23 15:41 ` [PATCH libselinux 07/11] Add more printf format annotations Daniel P. Berrange
2012-01-23 15:41 ` [PATCH libselinux 08/11] Add printf format attribute annotation to die() method Daniel P. Berrange
2012-01-23 15:41 ` [PATCH libselinux 09/11] Fix const-ness of parameters & make usage() methods static Daniel P. Berrange
2012-01-23 15:41 ` [PATCH libselinux 10/11] Enable many more gcc warnings for libselinux/src/ builds Daniel P. Berrange
2012-01-23 15:41 ` [PATCH libselinux 11/11] Enable many more gcc warnings for libselinux/utils builds Daniel P. Berrange
2012-01-23 16:30 ` [PATCH 00/11] Two bug fixes & misc code cleanup fixes Daniel J Walsh
2012-01-23 18:13 ` [PATCH 12/11] Change annotation on include/selinux/avc.h to avoid upsetting SWIG Daniel P. Berrange

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.