* [PATCH 2/3] libsemanage: initialize bools_modified variable.
2016-01-31 10:36 [PATCH 1/3] libsepol: cil: always initialize __cil_permx_to_sepol_class_perms() result Nicolas Iooss
@ 2016-01-31 10:36 ` Nicolas Iooss
2016-01-31 10:36 ` [PATCH 3/3] libsemanage: move modinfo_tmp definition before goto cleanup Nicolas Iooss
2016-02-01 14:14 ` [PATCH 1/3] libsepol: cil: always initialize __cil_permx_to_sepol_class_perms() result Steve Lawrence
2 siblings, 0 replies; 4+ messages in thread
From: Nicolas Iooss @ 2016-01-31 10:36 UTC (permalink / raw)
To: selinux
In semanage_direct_commit() error path, bools_modified can be used in a
if statement without being initialized (when a "goto cleanup" is taken
early). clang warns about this bug:
direct_api.c:1441:18: error: variable 'bools_modified' may be
uninitialized when used here [-Werror,-Wconditional-uninitialized]
if (modified || bools_modified) {
^~~~~~~~~~~~~~
direct_api.c:1087:48: note: initialize the variable 'bools_modified'
to silence this warning
preserve_tunables_modified, bools_modified,
^
= 0
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
---
libsemanage/src/direct_api.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c
index 68dd0d18425a..dd621d99295e 100644
--- a/libsemanage/src/direct_api.c
+++ b/libsemanage/src/direct_api.c
@@ -1076,7 +1076,7 @@ static int semanage_direct_commit(semanage_handle_t * sh)
/* Declare some variables */
int modified = 0, fcontexts_modified, ports_modified,
seusers_modified, users_extra_modified, dontaudit_modified,
- preserve_tunables_modified, bools_modified,
+ preserve_tunables_modified, bools_modified = 0,
disable_dontaudit, preserve_tunables;
dbase_config_t *users = semanage_user_dbase_local(sh);
dbase_config_t *users_base = semanage_user_base_dbase_local(sh);
--
2.7.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH 3/3] libsemanage: move modinfo_tmp definition before goto cleanup
2016-01-31 10:36 [PATCH 1/3] libsepol: cil: always initialize __cil_permx_to_sepol_class_perms() result Nicolas Iooss
2016-01-31 10:36 ` [PATCH 2/3] libsemanage: initialize bools_modified variable Nicolas Iooss
@ 2016-01-31 10:36 ` Nicolas Iooss
2016-02-01 14:14 ` [PATCH 1/3] libsepol: cil: always initialize __cil_permx_to_sepol_class_perms() result Steve Lawrence
2 siblings, 0 replies; 4+ messages in thread
From: Nicolas Iooss @ 2016-01-31 10:36 UTC (permalink / raw)
To: selinux
In semanage_direct_set_module_info() and semanage_direct_list_all()
functions, when modinfo_tmp variable gets initialized, a branch to
"cleanup" label may have already been taken. This leads to this
variable being possibly used uninitialized in these functions.
This is reported by clang:
direct_api.c:2491:41: error: variable 'modinfo_tmp' may be
uninitialized when used here [-Werror,-Wconditional-uninitialized]
ret = semanage_module_info_destroy(sh, modinfo_tmp);
^~~~~~~~~~~
direct_api.c:2334:2: note: variable 'modinfo_tmp' is declared here
semanage_module_info_t *modinfo_tmp = NULL;
^
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
---
libsemanage/src/direct_api.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c
index dd621d99295e..7c84bcea629b 100644
--- a/libsemanage/src/direct_api.c
+++ b/libsemanage/src/direct_api.c
@@ -2136,6 +2136,7 @@ static int semanage_direct_set_module_info(semanage_handle_t *sh,
char fn[PATH_MAX];
const char *path = NULL;
int enabled = 0;
+ semanage_module_info_t *modinfo_tmp = NULL;
semanage_module_key_t modkey;
ret = semanage_module_key_init(sh, &modkey);
@@ -2144,8 +2145,6 @@ static int semanage_direct_set_module_info(semanage_handle_t *sh,
goto cleanup;
}
- semanage_module_info_t *modinfo_tmp = NULL;
-
/* check transaction */
if (!sh->is_in_transaction) {
if (semanage_begin_transaction(sh) < 0) {
@@ -2316,6 +2315,8 @@ static int semanage_direct_list_all(semanage_handle_t *sh,
uint16_t priority = 0;
+ semanage_module_info_t *modinfo_tmp = NULL;
+
semanage_module_info_t modinfo;
ret = semanage_module_info_init(sh, &modinfo);
if (ret != 0) {
@@ -2323,8 +2324,6 @@ static int semanage_direct_list_all(semanage_handle_t *sh,
goto cleanup;
}
- semanage_module_info_t *modinfo_tmp = NULL;
-
if (sh->is_in_transaction) {
toplevel = semanage_path(SEMANAGE_TMP, SEMANAGE_MODULES);
} else {
--
2.7.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH 1/3] libsepol: cil: always initialize __cil_permx_to_sepol_class_perms() result
2016-01-31 10:36 [PATCH 1/3] libsepol: cil: always initialize __cil_permx_to_sepol_class_perms() result Nicolas Iooss
2016-01-31 10:36 ` [PATCH 2/3] libsemanage: initialize bools_modified variable Nicolas Iooss
2016-01-31 10:36 ` [PATCH 3/3] libsemanage: move modinfo_tmp definition before goto cleanup Nicolas Iooss
@ 2016-02-01 14:14 ` Steve Lawrence
2 siblings, 0 replies; 4+ messages in thread
From: Steve Lawrence @ 2016-02-01 14:14 UTC (permalink / raw)
To: Nicolas Iooss, selinux
On 01/31/2016 05:36 AM, Nicolas Iooss wrote:
> clang warns that __cil_permx_to_sepol_class_perms() return value, rc,
> may be unitialized:
>
> ../cil/src/cil_binary.c:4188:9: error: variable 'rc' may be
> uninitialized when used here [-Werror,-Wconditional-uninitialized]
> return rc;
> ^~
> ../cil/src/cil_binary.c:4148:8: note: initialize the variable 'rc'
> to silence this warning
> int rc;
> ^
> = 0
>
> This theoretically happens when cil_expand_class(permx->obj) returns an
> empty list.
>
> Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
All patches applied. Thanks!
- Steve
> ---
> libsepol/cil/src/cil_binary.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libsepol/cil/src/cil_binary.c b/libsepol/cil/src/cil_binary.c
> index 433f92e40a8e..47c751c0ef06 100644
> --- a/libsepol/cil/src/cil_binary.c
> +++ b/libsepol/cil/src/cil_binary.c
> @@ -4145,7 +4145,7 @@ exit:
>
> static int __cil_permx_to_sepol_class_perms(policydb_t *pdb, struct cil_permissionx *permx, class_perm_node_t **sepol_class_perms)
> {
> - int rc;
> + int rc = SEPOL_OK;
> struct cil_list *class_list = NULL;
> struct cil_list_item *c;
> class_datum_t *sepol_obj = NULL;
>
^ permalink raw reply [flat|nested] 4+ messages in thread