aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorArnd Bergmann <arnd@arndb.de>2017-09-15 15:55:46 -0400
committerJohn Johansen <john.johansen@canonical.com>2017-11-21 05:15:50 -0500
commit7bba39ae52c4d7b467d0a6f74cc067a561aac043 (patch)
tree3995a82c162d555bfdfc8901655fb19e852b0090
parent5933a62708fbae49931694314f3c98fbe91bb178 (diff)
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>
-rw-r--r--security/apparmor/file.c8
-rw-r--r--security/apparmor/lib.c13
-rw-r--r--security/apparmor/mount.c13
3 files changed, 12 insertions, 22 deletions
diff --git a/security/apparmor/file.c b/security/apparmor/file.c
index 3382518b87fa..e79bf44396a3 100644
--- a/security/apparmor/file.c
+++ b/security/apparmor/file.c
@@ -226,18 +226,12 @@ static u32 map_old_perms(u32 old)
226struct aa_perms aa_compute_fperms(struct aa_dfa *dfa, unsigned int state, 226struct aa_perms aa_compute_fperms(struct aa_dfa *dfa, unsigned int state,
227 struct path_cond *cond) 227 struct path_cond *cond)
228{ 228{
229 struct aa_perms perms;
230
231 /* FIXME: change over to new dfa format 229 /* FIXME: change over to new dfa format
232 * currently file perms are encoded in the dfa, new format 230 * currently file perms are encoded in the dfa, new format
233 * splits the permissions from the dfa. This mapping can be 231 * splits the permissions from the dfa. This mapping can be
234 * done at profile load 232 * done at profile load
235 */ 233 */
236 perms.deny = 0; 234 struct aa_perms perms = { };
237 perms.kill = perms.stop = 0;
238 perms.complain = perms.cond = 0;
239 perms.hide = 0;
240 perms.prompt = 0;
241 235
242 if (uid_eq(current_fsuid(), cond->uid)) { 236 if (uid_eq(current_fsuid(), cond->uid)) {
243 perms.allow = map_old_perms(dfa_user_allow(dfa, state)); 237 perms.allow = map_old_perms(dfa_user_allow(dfa, state));
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)
317void aa_compute_perms(struct aa_dfa *dfa, unsigned int state, 317void aa_compute_perms(struct aa_dfa *dfa, unsigned int state,
318 struct aa_perms *perms) 318 struct aa_perms *perms)
319{ 319{
320 perms->deny = 0; 320 *perms = (struct aa_perms) {
321 perms->kill = perms->stop = 0; 321 .allow = dfa_user_allow(dfa, state),
322 perms->complain = perms->cond = 0; 322 .audit = dfa_user_audit(dfa, state),
323 perms->hide = 0; 323 .quiet = dfa_user_quiet(dfa, state),
324 perms->prompt = 0; 324 };
325 perms->allow = dfa_user_allow(dfa, state);
326 perms->audit = dfa_user_audit(dfa, state);
327 perms->quiet = dfa_user_quiet(dfa, state);
328 325
329 /* for v5 perm mapping in the policydb, the other set is used 326 /* for v5 perm mapping in the policydb, the other set is used
330 * to extend the general perm set 327 * to extend the general perm set
diff --git a/security/apparmor/mount.c b/security/apparmor/mount.c
index 82a64b58041d..ed9b4d0f9f7e 100644
--- 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,
216static struct aa_perms compute_mnt_perms(struct aa_dfa *dfa, 216static struct aa_perms compute_mnt_perms(struct aa_dfa *dfa,
217 unsigned int state) 217 unsigned int state)
218{ 218{
219 struct aa_perms perms; 219 struct aa_perms perms = {
220 220 .allow = dfa_user_allow(dfa, state),
221 perms.kill = 0; 221 .audit = dfa_user_audit(dfa, state),
222 perms.allow = dfa_user_allow(dfa, state); 222 .quiet = dfa_user_quiet(dfa, state),
223 perms.audit = dfa_user_audit(dfa, state); 223 .xindex = dfa_user_xindex(dfa, state),
224 perms.quiet = dfa_user_quiet(dfa, state); 224 };
225 perms.xindex = dfa_user_xindex(dfa, state);
226 225
227 return perms; 226 return perms;
228} 227}