diff options
author | Linus Torvalds <torvalds@linux-foundation.org> | 2023-07-07 18:55:31 +0200 |
---|---|---|
committer | Linus Torvalds <torvalds@linux-foundation.org> | 2023-07-07 18:55:31 +0200 |
commit | 70806ee18a871140a73a2524a29865e1d904348c (patch) | |
tree | 27de3a7ef753f1370c30d7d79ab3934a2695e726 /security/apparmor | |
parent | Merge tag 'drm-next-2023-07-07' of git://anongit.freedesktop.org/drm/drm (diff) | |
parent | apparmor: Fix kernel-doc header for verify_dfa_accept_index (diff) | |
download | linux-70806ee18a871140a73a2524a29865e1d904348c.tar.xz linux-70806ee18a871140a73a2524a29865e1d904348c.zip |
Merge tag 'apparmor-pr-2023-07-06' of git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor
Pull apparmor updates from John Johansen:
- fix missing error check for rhashtable_insert_fast
- add missing failure check in compute_xmatch_perms
- fix policy_compat permission remap with extended permissions
- fix profile verification and enable it
- fix kzalloc perms tables for shared dfas
- Fix kernel-doc header for verify_dfa_accept_index
- aa_buffer: Convert 1-element array to flexible array
- Return directly after a failed kzalloc() in two functions
- fix use of strcpy in policy_unpack_test
- fix kernel-doc complaints
- Fix some kernel-doc comments
* tag 'apparmor-pr-2023-07-06' of git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor:
apparmor: Fix kernel-doc header for verify_dfa_accept_index
apparmor: fix: kzalloc perms tables for shared dfas
apparmor: fix profile verification and enable it
apparmor: fix policy_compat permission remap with extended permissions
apparmor: aa_buffer: Convert 1-element array to flexible array
apparmor: add missing failure check in compute_xmatch_perms
apparmor: fix missing error check for rhashtable_insert_fast
apparmor: Return directly after a failed kzalloc() in two functions
AppArmor: Fix some kernel-doc comments
apparmor: fix use of strcpy in policy_unpack_test
apparmor: fix kernel-doc complaints
Diffstat (limited to 'security/apparmor')
-rw-r--r-- | security/apparmor/crypto.c | 10 | ||||
-rw-r--r-- | security/apparmor/file.c | 2 | ||||
-rw-r--r-- | security/apparmor/lsm.c | 8 | ||||
-rw-r--r-- | security/apparmor/policy.c | 20 | ||||
-rw-r--r-- | security/apparmor/policy_compat.c | 20 | ||||
-rw-r--r-- | security/apparmor/policy_unpack.c | 102 | ||||
-rw-r--r-- | security/apparmor/policy_unpack_test.c | 13 | ||||
-rw-r--r-- | security/apparmor/secid.c | 3 |
8 files changed, 110 insertions, 68 deletions
diff --git a/security/apparmor/crypto.c b/security/apparmor/crypto.c index b498ed302461..6724e2ff6da8 100644 --- a/security/apparmor/crypto.c +++ b/security/apparmor/crypto.c @@ -28,15 +28,15 @@ unsigned int aa_hash_size(void) char *aa_calc_hash(void *data, size_t len) { SHASH_DESC_ON_STACK(desc, apparmor_tfm); - char *hash = NULL; - int error = -ENOMEM; + char *hash; + int error; if (!apparmor_tfm) return NULL; hash = kzalloc(apparmor_hash_size, GFP_KERNEL); if (!hash) - goto fail; + return ERR_PTR(-ENOMEM); desc->tfm = apparmor_tfm; @@ -62,7 +62,7 @@ int aa_calc_profile_hash(struct aa_profile *profile, u32 version, void *start, size_t len) { SHASH_DESC_ON_STACK(desc, apparmor_tfm); - int error = -ENOMEM; + int error; __le32 le32_version = cpu_to_le32(version); if (!aa_g_hash_policy) @@ -73,7 +73,7 @@ int aa_calc_profile_hash(struct aa_profile *profile, u32 version, void *start, profile->hash = kzalloc(apparmor_hash_size, GFP_KERNEL); if (!profile->hash) - goto fail; + return -ENOMEM; desc->tfm = apparmor_tfm; diff --git a/security/apparmor/file.c b/security/apparmor/file.c index 9119ddda6217..698b124e649f 100644 --- a/security/apparmor/file.c +++ b/security/apparmor/file.c @@ -161,6 +161,7 @@ static int path_name(const char *op, struct aa_label *label, return 0; } +struct aa_perms default_perms = {}; /** * aa_lookup_fperms - convert dfa compressed perms to internal perms * @dfa: dfa to lookup perms for (NOT NULL) @@ -171,7 +172,6 @@ static int path_name(const char *op, struct aa_label *label, * * Returns: a pointer to a file permission set */ -struct aa_perms default_perms = {}; struct aa_perms *aa_lookup_fperms(struct aa_policydb *file_rules, aa_state_t state, struct path_cond *cond) { diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index f431251ffb91..c9463bd0307d 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -46,7 +46,7 @@ int apparmor_initialized; union aa_buffer { struct list_head list; - char buffer[1]; + DECLARE_FLEX_ARRAY(char, buffer); }; #define RESERVE_COUNT 2 @@ -1647,7 +1647,7 @@ retry: list_del(&aa_buf->list); buffer_count--; spin_unlock(&aa_buffers_lock); - return &aa_buf->buffer[0]; + return aa_buf->buffer; } if (in_atomic) { /* @@ -1670,7 +1670,7 @@ retry: pr_warn_once("AppArmor: Failed to allocate a memory buffer.\n"); return NULL; } - return &aa_buf->buffer[0]; + return aa_buf->buffer; } void aa_put_buffer(char *buf) @@ -1747,7 +1747,7 @@ static int __init alloc_buffers(void) destroy_buffers(); return -ENOMEM; } - aa_put_buffer(&aa_buf->buffer[0]); + aa_put_buffer(aa_buf->buffer); } return 0; } diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c index 51e8184e0fec..b38f7b2a5e1d 100644 --- a/security/apparmor/policy.c +++ b/security/apparmor/policy.c @@ -430,11 +430,9 @@ static struct aa_policy *__lookup_parent(struct aa_ns *ns, * @hname: hierarchical profile name to find parent of (NOT NULL) * @gfp: type of allocation. * - * Returns: NULL on error, parent profile on success - * * Requires: ns mutex lock held * - * Returns: unrefcounted parent policy or NULL if error creating + * Return: unrefcounted parent policy on success or %NULL if error creating * place holder profiles. */ static struct aa_policy *__create_missing_ancestors(struct aa_ns *ns, @@ -591,7 +589,15 @@ struct aa_profile *aa_alloc_null(struct aa_profile *parent, const char *name, profile->label.flags |= FLAG_NULL; rules = list_first_entry(&profile->rules, typeof(*rules), list); rules->file.dfa = aa_get_dfa(nulldfa); + rules->file.perms = kcalloc(2, sizeof(struct aa_perms), GFP_KERNEL); + if (!rules->file.perms) + goto fail; + rules->file.size = 2; rules->policy.dfa = aa_get_dfa(nulldfa); + rules->policy.perms = kcalloc(2, sizeof(struct aa_perms), GFP_KERNEL); + if (!rules->policy.perms) + goto fail; + rules->policy.size = 2; if (parent) { profile->path_flags = parent->path_flags; @@ -602,6 +608,11 @@ struct aa_profile *aa_alloc_null(struct aa_profile *parent, const char *name, } return profile; + +fail: + aa_free_profile(profile); + + return NULL; } /** @@ -828,7 +839,7 @@ bool aa_current_policy_admin_capable(struct aa_ns *ns) /** * aa_may_manage_policy - can the current task manage policy * @label: label to check if it can manage policy - * @op: the policy manipulation operation being done + * @mask: contains the policy manipulation operation being done * * Returns: 0 if the task is allowed to manipulate policy else error */ @@ -883,7 +894,6 @@ static struct aa_profile *__list_lookup_parent(struct list_head *lh, * __replace_profile - replace @old with @new on a list * @old: profile to be replaced (NOT NULL) * @new: profile to replace @old with (NOT NULL) - * @share_proxy: transfer @old->proxy to @new * * Will duplicate and refcount elements that @new inherits from @old * and will inherit @old children. diff --git a/security/apparmor/policy_compat.c b/security/apparmor/policy_compat.c index cc89d1e88fb7..0cb02da8a319 100644 --- a/security/apparmor/policy_compat.c +++ b/security/apparmor/policy_compat.c @@ -146,7 +146,8 @@ static struct aa_perms compute_fperms_other(struct aa_dfa *dfa, * * Returns: remapped perm table */ -static struct aa_perms *compute_fperms(struct aa_dfa *dfa) +static struct aa_perms *compute_fperms(struct aa_dfa *dfa, + u32 *size) { aa_state_t state; unsigned int state_count; @@ -159,6 +160,7 @@ static struct aa_perms *compute_fperms(struct aa_dfa *dfa) table = kvcalloc(state_count * 2, sizeof(struct aa_perms), GFP_KERNEL); if (!table) return NULL; + *size = state_count * 2; for (state = 0; state < state_count; state++) { table[state * 2] = compute_fperms_user(dfa, state); @@ -168,7 +170,8 @@ static struct aa_perms *compute_fperms(struct aa_dfa *dfa) return table; } -static struct aa_perms *compute_xmatch_perms(struct aa_dfa *xmatch) +static struct aa_perms *compute_xmatch_perms(struct aa_dfa *xmatch, + u32 *size) { struct aa_perms *perms; int state; @@ -179,6 +182,9 @@ static struct aa_perms *compute_xmatch_perms(struct aa_dfa *xmatch) state_count = xmatch->tables[YYTD_ID_BASE]->td_lolen; /* DFAs are restricted from having a state_count of less than 2 */ perms = kvcalloc(state_count, sizeof(struct aa_perms), GFP_KERNEL); + if (!perms) + return NULL; + *size = state_count; /* zero init so skip the trap state (state == 0) */ for (state = 1; state < state_count; state++) @@ -239,7 +245,8 @@ static struct aa_perms compute_perms_entry(struct aa_dfa *dfa, return perms; } -static struct aa_perms *compute_perms(struct aa_dfa *dfa, u32 version) +static struct aa_perms *compute_perms(struct aa_dfa *dfa, u32 version, + u32 *size) { unsigned int state; unsigned int state_count; @@ -252,6 +259,7 @@ static struct aa_perms *compute_perms(struct aa_dfa *dfa, u32 version) table = kvcalloc(state_count, sizeof(struct aa_perms), GFP_KERNEL); if (!table) return NULL; + *size = state_count; /* zero init so skip the trap state (state == 0) */ for (state = 1; state < state_count; state++) @@ -286,7 +294,7 @@ static void remap_dfa_accept(struct aa_dfa *dfa, unsigned int factor) /* TODO: merge different dfa mappings into single map_policy fn */ int aa_compat_map_xmatch(struct aa_policydb *policy) { - policy->perms = compute_xmatch_perms(policy->dfa); + policy->perms = compute_xmatch_perms(policy->dfa, &policy->size); if (!policy->perms) return -ENOMEM; @@ -297,7 +305,7 @@ int aa_compat_map_xmatch(struct aa_policydb *policy) int aa_compat_map_policy(struct aa_policydb *policy, u32 version) { - policy->perms = compute_perms(policy->dfa, version); + policy->perms = compute_perms(policy->dfa, version, &policy->size); if (!policy->perms) return -ENOMEM; @@ -308,7 +316,7 @@ int aa_compat_map_policy(struct aa_policydb *policy, u32 version) int aa_compat_map_file(struct aa_policydb *policy) { - policy->perms = compute_fperms(policy->dfa); + policy->perms = compute_fperms(policy->dfa, &policy->size); if (!policy->perms) return -ENOMEM; diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c index cf2ceec40b28..694fb7a09962 100644 --- a/security/apparmor/policy_unpack.c +++ b/security/apparmor/policy_unpack.c @@ -448,7 +448,7 @@ static struct aa_dfa *unpack_dfa(struct aa_ext *e, int flags) /** * unpack_trans_table - unpack a profile transition table * @e: serialized data extent information (NOT NULL) - * @table: str table to unpack to (NOT NULL) + * @strs: str table to unpack to (NOT NULL) * * Returns: true if table successfully unpacked or not present */ @@ -860,10 +860,12 @@ 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; - error = aa_compat_map_xmatch(&profile->attach.xmatch); - if (error) { - info = "failed to convert xmatch permission table"; - goto fail; + if (!profile->attach.xmatch.perms) { + error = aa_compat_map_xmatch(&profile->attach.xmatch); + if (error) { + info = "failed to convert xmatch permission table"; + goto fail; + } } } @@ -983,31 +985,54 @@ static struct aa_profile *unpack_profile(struct aa_ext *e, char **ns_name) AA_CLASS_FILE); if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL)) goto fail; - error = aa_compat_map_policy(&rules->policy, e->version); - if (error) { - info = "failed to remap policydb permission table"; - goto fail; + if (!rules->policy.perms) { + error = aa_compat_map_policy(&rules->policy, + e->version); + if (error) { + info = "failed to remap policydb permission table"; + goto fail; + } } - } else + } else { rules->policy.dfa = aa_get_dfa(nulldfa); - + rules->policy.perms = kcalloc(2, sizeof(struct aa_perms), + GFP_KERNEL); + if (!rules->policy.perms) + goto fail; + rules->policy.size = 2; + } /* get file rules */ error = unpack_pdb(e, &rules->file, false, true, &info); if (error) { goto fail; } else if (rules->file.dfa) { - error = aa_compat_map_file(&rules->file); - if (error) { - info = "failed to remap file permission table"; - goto fail; + if (!rules->file.perms) { + error = aa_compat_map_file(&rules->file); + if (error) { + info = "failed to remap file permission table"; + goto fail; + } } } else if (rules->policy.dfa && rules->policy.start[AA_CLASS_FILE]) { rules->file.dfa = aa_get_dfa(rules->policy.dfa); rules->file.start[AA_CLASS_FILE] = rules->policy.start[AA_CLASS_FILE]; - } else + rules->file.perms = kcalloc(rules->policy.size, + sizeof(struct aa_perms), + GFP_KERNEL); + if (!rules->file.perms) + goto fail; + memcpy(rules->file.perms, rules->policy.perms, + rules->policy.size * sizeof(struct aa_perms)); + rules->file.size = rules->policy.size; + } else { rules->file.dfa = aa_get_dfa(nulldfa); - + rules->file.perms = kcalloc(2, sizeof(struct aa_perms), + GFP_KERNEL); + if (!rules->file.perms) + goto fail; + rules->file.size = 2; + } error = -EPROTO; if (aa_unpack_nameX(e, AA_STRUCT, "data")) { info = "out of memory"; @@ -1046,8 +1071,13 @@ static struct aa_profile *unpack_profile(struct aa_ext *e, char **ns_name) goto fail; } - rhashtable_insert_fast(profile->data, &data->head, - profile->data->p); + if (rhashtable_insert_fast(profile->data, &data->head, + profile->data->p)) { + kfree_sensitive(data->key); + kfree_sensitive(data); + info = "failed to insert data to table"; + goto fail; + } } if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL)) { @@ -1134,22 +1164,16 @@ static int verify_header(struct aa_ext *e, int required, const char **ns) return 0; } -static bool verify_xindex(int xindex, int table_size) -{ - int index, xtype; - xtype = xindex & AA_X_TYPE_MASK; - index = xindex & AA_X_INDEX_MASK; - if (xtype == AA_X_TABLE && index >= table_size) - return false; - return true; -} - -/* verify dfa xindexes are in range of transition tables */ -static bool verify_dfa_xindex(struct aa_dfa *dfa, int table_size) +/** + * verify_dfa_accept_index - verify accept indexes are in range of perms table + * @dfa: the dfa to check accept indexes are in range + * table_size: the permission table size the indexes should be within + */ +static bool verify_dfa_accept_index(struct aa_dfa *dfa, int table_size) { int i; for (i = 0; i < dfa->tables[YYTD_ID_ACCEPT]->td_lolen; i++) { - if (!verify_xindex(ACCEPT_TABLE(dfa)[i], table_size)) + if (ACCEPT_TABLE(dfa)[i] >= table_size) return false; } return true; @@ -1186,11 +1210,13 @@ static bool verify_perms(struct aa_policydb *pdb) if (!verify_perm(&pdb->perms[i])) return false; /* verify indexes into str table */ - if (pdb->perms[i].xindex >= pdb->trans.size) + if ((pdb->perms[i].xindex & AA_X_TYPE_MASK) == AA_X_TABLE && + (pdb->perms[i].xindex & AA_X_INDEX_MASK) >= pdb->trans.size) return false; - if (pdb->perms[i].tag >= pdb->trans.size) + if (pdb->perms[i].tag && pdb->perms[i].tag >= pdb->trans.size) return false; - if (pdb->perms[i].label >= pdb->trans.size) + if (pdb->perms[i].label && + pdb->perms[i].label >= pdb->trans.size) return false; } @@ -1212,10 +1238,10 @@ static int verify_profile(struct aa_profile *profile) if (!rules) return 0; - if ((rules->file.dfa && !verify_dfa_xindex(rules->file.dfa, - rules->file.trans.size)) || + if ((rules->file.dfa && !verify_dfa_accept_index(rules->file.dfa, + rules->file.size)) || (rules->policy.dfa && - !verify_dfa_xindex(rules->policy.dfa, rules->policy.trans.size))) { + !verify_dfa_accept_index(rules->policy.dfa, rules->policy.size))) { audit_iface(profile, NULL, NULL, "Unpack: Invalid named transition", NULL, -EPROTO); return -EPROTO; diff --git a/security/apparmor/policy_unpack_test.c b/security/apparmor/policy_unpack_test.c index e1bfdab524b7..5c9bde25e56d 100644 --- a/security/apparmor/policy_unpack_test.c +++ b/security/apparmor/policy_unpack_test.c @@ -69,31 +69,30 @@ static struct aa_ext *build_aa_ext_struct(struct policy_unpack_fixture *puf, *buf = AA_NAME; *(buf + 1) = strlen(TEST_STRING_NAME) + 1; - strcpy(buf + 3, TEST_STRING_NAME); + strscpy(buf + 3, TEST_STRING_NAME, e->end - (void *)(buf + 3)); buf = e->start + TEST_STRING_BUF_OFFSET; *buf = AA_STRING; *(buf + 1) = strlen(TEST_STRING_DATA) + 1; - strcpy(buf + 3, TEST_STRING_DATA); - + strscpy(buf + 3, TEST_STRING_DATA, e->end - (void *)(buf + 3)); buf = e->start + TEST_NAMED_U32_BUF_OFFSET; *buf = AA_NAME; *(buf + 1) = strlen(TEST_U32_NAME) + 1; - strcpy(buf + 3, TEST_U32_NAME); + strscpy(buf + 3, TEST_U32_NAME, e->end - (void *)(buf + 3)); *(buf + 3 + strlen(TEST_U32_NAME) + 1) = AA_U32; *((u32 *)(buf + 3 + strlen(TEST_U32_NAME) + 2)) = TEST_U32_DATA; buf = e->start + TEST_NAMED_U64_BUF_OFFSET; *buf = AA_NAME; *(buf + 1) = strlen(TEST_U64_NAME) + 1; - strcpy(buf + 3, TEST_U64_NAME); + strscpy(buf + 3, TEST_U64_NAME, e->end - (void *)(buf + 3)); *(buf + 3 + strlen(TEST_U64_NAME) + 1) = AA_U64; *((u64 *)(buf + 3 + strlen(TEST_U64_NAME) + 2)) = TEST_U64_DATA; buf = e->start + TEST_NAMED_BLOB_BUF_OFFSET; *buf = AA_NAME; *(buf + 1) = strlen(TEST_BLOB_NAME) + 1; - strcpy(buf + 3, TEST_BLOB_NAME); + strscpy(buf + 3, TEST_BLOB_NAME, e->end - (void *)(buf + 3)); *(buf + 3 + strlen(TEST_BLOB_NAME) + 1) = AA_BLOB; *(buf + 3 + strlen(TEST_BLOB_NAME) + 2) = TEST_BLOB_DATA_SIZE; memcpy(buf + 3 + strlen(TEST_BLOB_NAME) + 6, @@ -102,7 +101,7 @@ static struct aa_ext *build_aa_ext_struct(struct policy_unpack_fixture *puf, buf = e->start + TEST_NAMED_ARRAY_BUF_OFFSET; *buf = AA_NAME; *(buf + 1) = strlen(TEST_ARRAY_NAME) + 1; - strcpy(buf + 3, TEST_ARRAY_NAME); + strscpy(buf + 3, TEST_ARRAY_NAME, e->end - (void *)(buf + 3)); *(buf + 3 + strlen(TEST_ARRAY_NAME) + 1) = AA_ARRAY; *((u16 *)(buf + 3 + strlen(TEST_ARRAY_NAME) + 2)) = TEST_ARRAY_SIZE; diff --git a/security/apparmor/secid.c b/security/apparmor/secid.c index 24a0e23f1b2b..83d3d1e6d9dc 100644 --- a/security/apparmor/secid.c +++ b/security/apparmor/secid.c @@ -53,8 +53,7 @@ void aa_secid_update(u32 secid, struct aa_label *label) xa_unlock_irqrestore(&aa_secids, flags); } -/** - * +/* * see label for inverse aa_label_to_secid */ struct aa_label *aa_secid_to_label(u32 secid) |