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 <arnd@arndb.de>
Reviewed-by: Seth Arnold <seth.arnold@canonical.com>
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: John Johansen <john.johansen@canonical.com>

diff --git a/security/apparmor/file.c b/security/apparmor/file.c
index 3382518b87fa507200679cb9ef660329a292debc..e79bf44396a36f60dde2e17fc68f2d0ac7d914b9 100644 (file)
--- a/security/apparmor/file.c
+++ b/security/apparmor/file.c
@@ -226,18 +226,12 @@ static u32 map_old_perms(u32 old)
 struct aa_perms aa_compute_fperms(struct aa_dfa *dfa, unsigned int state,
                                  struct path_cond *cond)
 {
-       struct aa_perms perms;
-
        /* FIXME: change over to new dfa format
         * currently file perms are encoded in the dfa, new format
         * splits the permissions from the dfa.  This mapping can be
         * done at profile load
         */
-       perms.deny = 0;
-       perms.kill = perms.stop = 0;
-       perms.complain = perms.cond = 0;
-       perms.hide = 0;
-       perms.prompt = 0;
+       struct aa_perms perms = { };
 
        if (uid_eq(current_fsuid(), cond->uid)) {
                perms.allow = map_old_perms(dfa_user_allow(dfa, state));

diff --git a/security/apparmor/lib.c b/security/apparmor/lib.c
index 08ca26bcca7703c7f74e1531879eea4dd3bf2ac9..3d0a2bf87abdf9c258c5816b695544b7ddc4e8c1 100644 (file)
--- 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

diff --git a/security/apparmor/mount.c b/security/apparmor/mount.c
index 82a64b58041d2adc62debcf572b77b7a6607678d..ed9b4d0f9f7e212b161c1a312c52b56f5e0b0b49 100644 (file)
--- a/security/apparmor/mount.c
+++ b/security/apparmor/mount.c
@@ -216,13 +216,12 @@ static unsigned int match_mnt_flags(struct aa_dfa *dfa, unsigned int state,
 static struct aa_perms compute_mnt_perms(struct aa_dfa *dfa,
                                           unsigned int state)
 {
-       struct aa_perms perms;
-
-       perms.kill = 0;
-       perms.allow = dfa_user_allow(dfa, state);
-       perms.audit = dfa_user_audit(dfa, state);
-       perms.quiet = dfa_user_quiet(dfa, state);
-       perms.xindex = dfa_user_xindex(dfa, state);
+       struct aa_perms perms = {
+               .allow = dfa_user_allow(dfa, state),
+               .audit = dfa_user_audit(dfa, state),
+               .quiet = dfa_user_quiet(dfa, state),
+               .xindex = dfa_user_xindex(dfa, state),
+       };
 
        return perms;
 }