Skip to content

Commit

Permalink
apparmor: Fix unpack_profile() warn: passing zero to 'ERR_PTR'
Browse files Browse the repository at this point in the history
unpack_profile() sets a default error on entry but this gets overridden
by error assignment by functions called in its body. If an error
check that was relying on the default value is triggered after one
of these error assignments then zero will be passed to ERR_PTR.

Fix this by setting up a default -EPROTO assignment in the error
path and while we are at it make sure the correct error is returned
in non-default cases.

Fixes: 217af7e ("apparmor: refactor profile rules and attachments")
Reported-by: kernel test robot <[email protected]>
Signed-off-by: John Johansen <[email protected]>
  • Loading branch information
John Johansen committed Oct 11, 2022
1 parent ee21a17 commit 53991ae
Showing 1 changed file with 16 additions and 5 deletions.
21 changes: 16 additions & 5 deletions security/apparmor/policy_unpack.c
Original file line number Diff line number Diff line change
Expand Up @@ -851,6 +851,7 @@ static struct aa_profile *unpack_profile(struct aa_ext *e, char **ns_name)
*ns_name = kstrndup(tmpns, ns_len, GFP_KERNEL);
if (!*ns_name) {
info = "out of memory";
error = -ENOMEM;
goto fail;
}
name = tmpname;
Expand Down Expand Up @@ -882,7 +883,8 @@ static struct aa_profile *unpack_profile(struct aa_ext *e, char **ns_name)
}
profile->attach.xmatch_len = tmp;
profile->attach.xmatch.start[AA_CLASS_XMATCH] = DFA_START;
if (aa_compat_map_xmatch(&profile->attach.xmatch)) {
error = aa_compat_map_xmatch(&profile->attach.xmatch);
if (error) {
info = "failed to convert xmatch permission table";
goto fail;
}
Expand Down Expand Up @@ -1004,7 +1006,8 @@ static struct aa_profile *unpack_profile(struct aa_ext *e, char **ns_name)
AA_CLASS_FILE);
if (!unpack_nameX(e, AA_STRUCTEND, NULL))
goto fail;
if (aa_compat_map_policy(&rules->policy, e->version)) {
error = aa_compat_map_policy(&rules->policy, e->version);
if (error) {
info = "failed to remap policydb permission table";
goto fail;
}
Expand All @@ -1016,7 +1019,8 @@ static struct aa_profile *unpack_profile(struct aa_ext *e, char **ns_name)
if (error) {
goto fail;
} else if (rules->file.dfa) {
if (aa_compat_map_file(&rules->file)) {
error = aa_compat_map_file(&rules->file);
if (error) {
info = "failed to remap file permission table";
goto fail;
}
Expand All @@ -1027,12 +1031,14 @@ static struct aa_profile *unpack_profile(struct aa_ext *e, char **ns_name)
} else
rules->file.dfa = aa_get_dfa(nulldfa);

error = -EPROTO;
if (unpack_nameX(e, AA_STRUCT, "data")) {
info = "out of memory";
profile->data = kzalloc(sizeof(*profile->data), GFP_KERNEL);
if (!profile->data)
if (!profile->data) {
error = -ENOMEM;
goto fail;

}
params.nelem_hint = 3;
params.key_len = sizeof(void *);
params.key_offset = offsetof(struct aa_data, key);
Expand All @@ -1049,6 +1055,7 @@ static struct aa_profile *unpack_profile(struct aa_ext *e, char **ns_name)
data = kzalloc(sizeof(*data), GFP_KERNEL);
if (!data) {
kfree_sensitive(key);
error = -ENOMEM;
goto fail;
}

Expand All @@ -1058,6 +1065,7 @@ static struct aa_profile *unpack_profile(struct aa_ext *e, char **ns_name)
if (data->size && !data->data) {
kfree_sensitive(data->key);
kfree_sensitive(data);
error = -ENOMEM;
goto fail;
}

Expand All @@ -1079,6 +1087,9 @@ static struct aa_profile *unpack_profile(struct aa_ext *e, char **ns_name)
return profile;

fail:
if (error == 0)
/* default error covers most cases */
error = -EPROTO;
if (profile)
name = NULL;
else if (!name)
Expand Down

0 comments on commit 53991ae

Please sign in to comment.