From 7bba39ae52c4d7b467d0a6f74cc067a561aac043 Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Fri, 15 Sep 2017 21:55:46 +0200 Subject: apparmor: initialized returned struct aa_perms gcc-4.4 points out suspicious code in compute_mnt_perms, where the aa_perms structure is only partially initialized before getting returned: security/apparmor/mount.c: In function 'compute_mnt_perms': security/apparmor/mount.c:227: error: 'perms.prompt' is used uninitialized in this function security/apparmor/mount.c:227: error: 'perms.hide' is used uninitialized in this function security/apparmor/mount.c:227: error: 'perms.cond' is used uninitialized in this function security/apparmor/mount.c:227: error: 'perms.complain' is used uninitialized in this function security/apparmor/mount.c:227: error: 'perms.stop' is used uninitialized in this function security/apparmor/mount.c:227: error: 'perms.deny' is used uninitialized in this function Returning or assigning partially initialized structures is a bit tricky, in particular it is explicitly allowed in c99 to assign a partially initialized structure to another, as long as only members are read that have been initialized earlier. Looking at what various compilers do here, the version that produced the warning copied uninitialized stack data, while newer versions (and also clang) either set the other members to zero or don't update the parts of the return buffer that are not modified in the temporary structure, but they never warn about this. In case of apparmor, it seems better to be a little safer and always initialize the aa_perms structure. Most users already do that, this changes the remaining ones, including the one instance that I got the warning for. Fixes: fa488437d0f9 ("apparmor: add mount mediation") Signed-off-by: Arnd Bergmann Reviewed-by: Seth Arnold Acked-by: Geert Uytterhoeven Signed-off-by: John Johansen --- security/apparmor/lib.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) (limited to 'security/apparmor/lib.c') diff --git a/security/apparmor/lib.c b/security/apparmor/lib.c index 08ca26bcca77..3d0a2bf87abd 100644 --- a/security/apparmor/lib.c +++ b/security/apparmor/lib.c @@ -317,14 +317,11 @@ static u32 map_other(u32 x) void aa_compute_perms(struct aa_dfa *dfa, unsigned int state, struct aa_perms *perms) { - perms->deny = 0; - perms->kill = perms->stop = 0; - perms->complain = perms->cond = 0; - perms->hide = 0; - perms->prompt = 0; - perms->allow = dfa_user_allow(dfa, state); - perms->audit = dfa_user_audit(dfa, state); - perms->quiet = dfa_user_quiet(dfa, state); + *perms = (struct aa_perms) { + .allow = dfa_user_allow(dfa, state), + .audit = dfa_user_audit(dfa, state), + .quiet = dfa_user_quiet(dfa, state), + }; /* for v5 perm mapping in the policydb, the other set is used * to extend the general perm set -- cgit v1.2.3-59-g8ed1b From e3bcfc148588e409685479f3d20ba3d66ae30035 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Sat, 14 Oct 2017 13:14:38 +0100 Subject: apparmor: remove unused redundant variable stop The boolean variable 'stop' is being set but never read. This is a redundant variable and can be removed. Cleans up clang warning: Value stored to 'stop' is never read Signed-off-by: Colin Ian King Signed-off-by: John Johansen --- security/apparmor/lib.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'security/apparmor/lib.c') diff --git a/security/apparmor/lib.c b/security/apparmor/lib.c index 3d0a2bf87abd..4d5e98e49d5e 100644 --- a/security/apparmor/lib.c +++ b/security/apparmor/lib.c @@ -423,7 +423,6 @@ int aa_check_perms(struct aa_profile *profile, struct aa_perms *perms, void (*cb)(struct audit_buffer *, void *)) { int type, error; - bool stop = false; u32 denied = request & (~perms->allow | perms->deny); if (likely(!denied)) { @@ -444,8 +443,6 @@ int aa_check_perms(struct aa_profile *profile, struct aa_perms *perms, else type = AUDIT_APPARMOR_DENIED; - if (denied & perms->stop) - stop = true; if (denied == (denied & perms->hide)) error = -ENOENT; -- cgit v1.2.3-59-g8ed1b