From: Linus Torvalds Date: Wed, 14 Dec 2022 21:42:09 +0000 (-0800) Subject: Merge tag 'apparmor-pr-2022-12-14' of git://git.kernel.org/pub/scm/linux/kernel/git... X-Git-Tag: v6.2-rc1~86 X-Git-Url: https://git.kernel.dk/?a=commitdiff_plain;h=93761c93e9da28d8a020777cee2a84133082b477;p=linux-block.git Merge tag 'apparmor-pr-2022-12-14' of git://git./linux/kernel/git/jj/linux-apparmor Pull apparmor updates from John Johansen: "Features: - switch to zstd compression for profile raw data Cleanups: - simplify obtaining the newest label on a cred - remove useless static inline functions - compute permission conversion on policy unpack - refactor code to share common permissins - refactor unpack to group policy backwards compatiblity code - add __init annotation to aa_{setup/teardown}_dfa_engine() Bug Fixes: - fix a memleak in - multi_transaction_new() - free_ruleset() - unpack_profile() - alloc_ns() - fix lockdep warning when removing a namespace - fix regression in stacking due to label flags - fix loading of child before parent - fix kernel-doc comments that differ from fns - fix spelling errors in comments - store return value of unpack_perms_table() to signed variable" * tag 'apparmor-pr-2022-12-14' of git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor: (64 commits) apparmor: Fix uninitialized symbol 'array_size' in policy_unpack_test.c apparmor: Add __init annotation to aa_{setup/teardown}_dfa_engine() apparmor: Fix memleak in alloc_ns() apparmor: Fix memleak issue in unpack_profile() apparmor: fix a memleak in free_ruleset() apparmor: Fix spelling of function name in comment block apparmor: Use pointer to struct aa_label for lbs_cred AppArmor: Fix kernel-doc LSM: Fix kernel-doc AppArmor: Fix kernel-doc apparmor: Fix loading of child before parent apparmor: refactor code that alloc null profiles apparmor: fix obsoleted comments for aa_getprocattr() and audit_resource() apparmor: remove useless static inline functions apparmor: Fix unpack_profile() warn: passing zero to 'ERR_PTR' apparmor: fix uninitialize table variable in error in unpack_trans_table apparmor: store return value of unpack_perms_table() to signed variable apparmor: Fix kunit test for out of bounds array apparmor: Fix decompression of rawdata for read back to userspace apparmor: Fix undefined references to zstd_ symbols ... --- 93761c93e9da28d8a020777cee2a84133082b477 diff --cc security/apparmor/Makefile index 065f4e346553,4377123c2b98..b9c5879dd599 --- a/security/apparmor/Makefile +++ b/security/apparmor/Makefile @@@ -5,12 -5,10 +5,13 @@@ obj-$(CONFIG_SECURITY_APPARMOR) += appa apparmor-y := apparmorfs.o audit.o capability.o task.o ipc.o lib.o match.o \ path.o domain.o policy.o policy_unpack.o procattr.o lsm.o \ - resource.o secid.o file.o policy_ns.o label.o mount.o net.o + resource.o secid.o file.o policy_ns.o label.o mount.o net.o \ + policy_compat.o apparmor-$(CONFIG_SECURITY_APPARMOR_HASH) += crypto.o +obj-$(CONFIG_SECURITY_APPARMOR_KUNIT_TEST) += apparmor_policy_unpack_test.o +apparmor_policy_unpack_test-objs += policy_unpack_test.o + clean-files := capability_names.h rlim_names.h net_names.h # Build a lower case string table of address family names diff --cc security/apparmor/domain.c index 00dc0ec066de,b447bc13ea8e..6dd3cc5309bf --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@@ -308,14 -296,16 +296,15 @@@ static int change_profile_perms(struct * Returns: number of extended attributes that matched, or < 0 on error */ static int aa_xattrs_match(const struct linux_binprm *bprm, - struct aa_profile *profile, unsigned int state) + struct aa_profile *profile, aa_state_t state) { int i; - ssize_t size; struct dentry *d; char *value = NULL; - int size, value_size = 0, ret = profile->xattr_count; + struct aa_attachment *attach = &profile->attach; - int value_size = 0, ret = attach->xattr_count; ++ int size, value_size = 0, ret = attach->xattr_count; - if (!bprm || !profile->xattr_count) + if (!bprm || !attach->xattr_count) return 0; might_sleep(); diff --cc security/apparmor/include/policy_unpack.h index e89b701447bc,1e10e360a0ec..a6f4611ee50c --- a/security/apparmor/include/policy_unpack.h +++ b/security/apparmor/include/policy_unpack.h @@@ -163,17 -128,4 +165,17 @@@ static inline void aa_put_loaddata(stru kref_put(&data->count, aa_loaddata_kref); } +#if IS_ENABLED(CONFIG_KUNIT) +bool aa_inbounds(struct aa_ext *e, size_t size); +size_t aa_unpack_u16_chunk(struct aa_ext *e, char **chunk); +bool aa_unpack_X(struct aa_ext *e, enum aa_code code); +bool aa_unpack_nameX(struct aa_ext *e, enum aa_code code, const char *name); +bool aa_unpack_u32(struct aa_ext *e, u32 *data, const char *name); +bool aa_unpack_u64(struct aa_ext *e, u64 *data, const char *name); - size_t aa_unpack_array(struct aa_ext *e, const char *name); ++bool aa_unpack_array(struct aa_ext *e, const char *name, u16 *size); +size_t aa_unpack_blob(struct aa_ext *e, char **blob, const char *name); +int aa_unpack_str(struct aa_ext *e, const char **string, const char *name); +int aa_unpack_strdup(struct aa_ext *e, char **string, const char *name); +#endif + #endif /* __POLICY_INTERFACE_H */ diff --cc security/apparmor/policy_unpack.c index 12e535fdfa8b,1bf8cfb8700a..66915653108c --- a/security/apparmor/policy_unpack.c +++ b/security/apparmor/policy_unpack.c @@@ -14,10 -14,9 +14,10 @@@ */ #include +#include #include #include - #include + #include #include "include/apparmor.h" #include "include/audit.h" @@@ -27,17 -27,51 +28,8 @@@ #include "include/path.h" #include "include/policy.h" #include "include/policy_unpack.h" - - #define K_ABI_MASK 0x3ff - #define FORCE_COMPLAIN_FLAG 0x800 - #define VERSION_LT(X, Y) (((X) & K_ABI_MASK) < ((Y) & K_ABI_MASK)) - #define VERSION_GT(X, Y) (((X) & K_ABI_MASK) > ((Y) & K_ABI_MASK)) - - #define v5 5 /* base version */ - #define v6 6 /* per entry policydb mediation check */ - #define v7 7 - #define v8 8 /* full network masking */ + #include "include/policy_compat.h" - -/* - * The AppArmor interface treats data as a type byte followed by the - * actual data. The interface has the notion of a named entry - * which has a name (AA_NAME typecode followed by name string) followed by - * the entries typecode and data. Named types allow for optional - * elements and extensions to be added and tested for without breaking - * backwards compatibility. - */ - -enum aa_code { - AA_U8, - AA_U16, - AA_U32, - AA_U64, - AA_NAME, /* same as string except it is items name */ - AA_STRING, - AA_BLOB, - AA_STRUCT, - AA_STRUCTEND, - AA_LIST, - AA_LISTEND, - AA_ARRAY, - AA_ARRAYEND, -}; - -/* - * aa_ext is the read of the buffer containing the serialized profile. The - * data is copied into a kernel buffer in apparmorfs and then handed off to - * the unpack routines. - */ -struct aa_ext { - void *start; - void *end; - void *pos; /* pointer to current position in the buffer */ - u32 version; -}; - -#define tri int -#define TRI_TRUE 1 -#define TRI_NONE 0 -#define TRI_FALSE -1 - /* audit callback for unpack fields */ static void audit_cb(struct audit_buffer *ab, void *va) { @@@ -319,28 -348,26 +311,27 @@@ fail e->pos = pos; return false; } +EXPORT_SYMBOL_IF_KUNIT(aa_unpack_u64); - VISIBLE_IF_KUNIT size_t aa_unpack_array(struct aa_ext *e, const char *name) -static tri unpack_array(struct aa_ext *e, const char *name, u16 *size) ++VISIBLE_IF_KUNIT bool aa_unpack_array(struct aa_ext *e, const char *name, u16 *size) { void *pos = e->pos; - if (unpack_nameX(e, AA_ARRAY, name)) { - if (!inbounds(e, sizeof(u16))) + if (aa_unpack_nameX(e, AA_ARRAY, name)) { - int size; + if (!aa_inbounds(e, sizeof(u16))) goto fail; - size = (int)le16_to_cpu(get_unaligned((__le16 *) e->pos)); + *size = le16_to_cpu(get_unaligned((__le16 *) e->pos)); e->pos += sizeof(u16); - return size; - return TRI_TRUE; ++ return true; } - return TRI_NONE; fail: e->pos = pos; - return 0; - return TRI_FALSE; ++ return false; } +EXPORT_SYMBOL_IF_KUNIT(aa_unpack_array); -static size_t unpack_blob(struct aa_ext *e, char **blob, const char *name) +VISIBLE_IF_KUNIT size_t aa_unpack_blob(struct aa_ext *e, char **blob, const char *name) { void *pos = e->pos; @@@ -447,32 -470,36 +437,36 @@@ static struct aa_dfa *unpack_dfa(struc /** * unpack_trans_table - unpack a profile transition table * @e: serialized data extent information (NOT NULL) - * @profile: profile to add the accept table to (NOT NULL) + * @table: str table to unpack to (NOT NULL) * - * Returns: true if table successfully unpacked + * Returns: true if table successfully unpacked or not present */ - static bool unpack_trans_table(struct aa_ext *e, struct aa_profile *profile) + static bool unpack_trans_table(struct aa_ext *e, struct aa_str_table *strs) { void *saved_pos = e->pos; + char **table = NULL; /* exec table is optional */ - if (unpack_nameX(e, AA_STRUCT, "xtable")) { + if (aa_unpack_nameX(e, AA_STRUCT, "xtable")) { - int i, size; - - size = aa_unpack_array(e, NULL); - /* currently 4 exec bits and entries 0-3 are reserved iupcx */ - if (size > 16 - 4) + u16 size; + int i; + - if (unpack_array(e, NULL, &size) != TRI_TRUE) ++ if (!aa_unpack_array(e, NULL, &size)) + /* + * Note: index into trans table array is a max + * of 2^24, but unpack array can only unpack + * an array of 2^16 in size atm so no need + * for size check here + */ goto fail; - profile->file.trans.table = kcalloc(size, sizeof(char *), - GFP_KERNEL); - if (!profile->file.trans.table) + table = kcalloc(size, sizeof(char *), GFP_KERNEL); + if (!table) goto fail; - profile->file.trans.size = size; for (i = 0; i < size; i++) { char *str; - int c, j, pos, size2 = unpack_strdup(e, &str, NULL); - /* unpack_strdup verifies that the last character is + int c, j, pos, size2 = aa_unpack_strdup(e, &str, NULL); + /* aa_unpack_strdup verifies that the last character is * null termination byte. */ if (!size2) @@@ -507,10 -534,13 +501,13 @@@ /* fail - all other cases with embedded \0 */ goto fail; } - if (!unpack_nameX(e, AA_ARRAYEND, NULL)) + if (!aa_unpack_nameX(e, AA_ARRAYEND, NULL)) goto fail; - if (!unpack_nameX(e, AA_STRUCTEND, NULL)) + if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL)) goto fail; + + strs->table = table; + strs->size = size; } return true; @@@ -524,21 -554,23 +521,23 @@@ static bool unpack_xattrs(struct aa_ex { void *pos = e->pos; - if (unpack_nameX(e, AA_STRUCT, "xattrs")) { + if (aa_unpack_nameX(e, AA_STRUCT, "xattrs")) { - int i, size; + u16 size; + int i; - size = aa_unpack_array(e, NULL); - profile->xattr_count = size; - profile->xattrs = kcalloc(size, sizeof(char *), GFP_KERNEL); - if (!profile->xattrs) - if (unpack_array(e, NULL, &size) != TRI_TRUE) ++ if (!aa_unpack_array(e, NULL, &size)) + goto fail; + profile->attach.xattr_count = size; + profile->attach.xattrs = kcalloc(size, sizeof(char *), GFP_KERNEL); + if (!profile->attach.xattrs) goto fail; for (i = 0; i < size; i++) { - if (!aa_unpack_strdup(e, &profile->xattrs[i], NULL)) - if (!unpack_strdup(e, &profile->attach.xattrs[i], NULL)) ++ if (!aa_unpack_strdup(e, &profile->attach.xattrs[i], NULL)) goto fail; } - if (!unpack_nameX(e, AA_ARRAYEND, NULL)) + if (!aa_unpack_nameX(e, AA_ARRAYEND, NULL)) goto fail; - if (!unpack_nameX(e, AA_STRUCTEND, NULL)) + if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL)) goto fail; } @@@ -549,32 -581,34 +548,34 @@@ fail return false; } - static bool unpack_secmark(struct aa_ext *e, struct aa_profile *profile) + static bool unpack_secmark(struct aa_ext *e, struct aa_ruleset *rules) { void *pos = e->pos; - int i, size; + u16 size; + int i; - if (unpack_nameX(e, AA_STRUCT, "secmark")) { - if (unpack_array(e, NULL, &size) != TRI_TRUE) + if (aa_unpack_nameX(e, AA_STRUCT, "secmark")) { - size = aa_unpack_array(e, NULL); ++ if (!aa_unpack_array(e, NULL, &size)) + goto fail; - profile->secmark = kcalloc(size, sizeof(struct aa_secmark), + rules->secmark = kcalloc(size, sizeof(struct aa_secmark), GFP_KERNEL); - if (!profile->secmark) + if (!rules->secmark) goto fail; - profile->secmark_count = size; + rules->secmark_count = size; for (i = 0; i < size; i++) { - if (!unpack_u8(e, &profile->secmark[i].audit, NULL)) + if (!unpack_u8(e, &rules->secmark[i].audit, NULL)) goto fail; - if (!unpack_u8(e, &profile->secmark[i].deny, NULL)) + if (!unpack_u8(e, &rules->secmark[i].deny, NULL)) goto fail; - if (!aa_unpack_strdup(e, &profile->secmark[i].label, NULL)) - if (!unpack_strdup(e, &rules->secmark[i].label, NULL)) ++ if (!aa_unpack_strdup(e, &rules->secmark[i].label, NULL)) goto fail; } - if (!unpack_nameX(e, AA_ARRAYEND, NULL)) + if (!aa_unpack_nameX(e, AA_ARRAYEND, NULL)) goto fail; - if (!unpack_nameX(e, AA_STRUCTEND, NULL)) + if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL)) goto fail; } @@@ -598,26 -632,27 +599,27 @@@ static bool unpack_rlimits(struct aa_ex void *pos = e->pos; /* rlimits are optional */ - if (unpack_nameX(e, AA_STRUCT, "rlimits")) { + if (aa_unpack_nameX(e, AA_STRUCT, "rlimits")) { - int i, size; + u16 size; + int i; u32 tmp = 0; - if (!unpack_u32(e, &tmp, NULL)) + if (!aa_unpack_u32(e, &tmp, NULL)) goto fail; - profile->rlimits.mask = tmp; + rules->rlimits.mask = tmp; - size = aa_unpack_array(e, NULL); - if (size > RLIM_NLIMITS) - if (unpack_array(e, NULL, &size) != TRI_TRUE || ++ if (!aa_unpack_array(e, NULL, &size) || + size > RLIM_NLIMITS) goto fail; for (i = 0; i < size; i++) { u64 tmp2 = 0; int a = aa_map_resource(i); - if (!unpack_u64(e, &tmp2, NULL)) + if (!aa_unpack_u64(e, &tmp2, NULL)) goto fail; - profile->rlimits.limits[a].rlim_max = tmp2; + rules->rlimits.limits[a].rlim_max = tmp2; } - if (!unpack_nameX(e, AA_ARRAYEND, NULL)) + if (!aa_unpack_nameX(e, AA_ARRAYEND, NULL)) goto fail; - if (!unpack_nameX(e, AA_STRUCTEND, NULL)) + if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL)) goto fail; } return true; @@@ -627,6 -662,144 +629,140 @@@ fail return false; } + static bool unpack_perm(struct aa_ext *e, u32 version, struct aa_perms *perm) + { - bool res; - + if (version != 1) + return false; + - res = unpack_u32(e, &perm->allow, NULL); - res = res && unpack_u32(e, &perm->allow, NULL); - res = res && unpack_u32(e, &perm->deny, NULL); - res = res && unpack_u32(e, &perm->subtree, NULL); - res = res && unpack_u32(e, &perm->cond, NULL); - res = res && unpack_u32(e, &perm->kill, NULL); - res = res && unpack_u32(e, &perm->complain, NULL); - res = res && unpack_u32(e, &perm->prompt, NULL); - res = res && unpack_u32(e, &perm->audit, NULL); - res = res && unpack_u32(e, &perm->quiet, NULL); - res = res && unpack_u32(e, &perm->hide, NULL); - res = res && unpack_u32(e, &perm->xindex, NULL); - res = res && unpack_u32(e, &perm->tag, NULL); - res = res && unpack_u32(e, &perm->label, NULL); - - return res; ++ return aa_unpack_u32(e, &perm->allow, NULL) && ++ aa_unpack_u32(e, &perm->allow, NULL) && ++ aa_unpack_u32(e, &perm->deny, NULL) && ++ aa_unpack_u32(e, &perm->subtree, NULL) && ++ aa_unpack_u32(e, &perm->cond, NULL) && ++ aa_unpack_u32(e, &perm->kill, NULL) && ++ aa_unpack_u32(e, &perm->complain, NULL) && ++ aa_unpack_u32(e, &perm->prompt, NULL) && ++ aa_unpack_u32(e, &perm->audit, NULL) && ++ aa_unpack_u32(e, &perm->quiet, NULL) && ++ aa_unpack_u32(e, &perm->hide, NULL) && ++ aa_unpack_u32(e, &perm->xindex, NULL) && ++ aa_unpack_u32(e, &perm->tag, NULL) && ++ aa_unpack_u32(e, &perm->label, NULL); + } + + static ssize_t unpack_perms_table(struct aa_ext *e, struct aa_perms **perms) + { + void *pos = e->pos; + u16 size = 0; + + AA_BUG(!perms); + /* + * policy perms are optional, in which case perms are embedded + * in the dfa accept table + */ - if (unpack_nameX(e, AA_STRUCT, "perms")) { ++ if (aa_unpack_nameX(e, AA_STRUCT, "perms")) { + int i; + u32 version; + - if (!unpack_u32(e, &version, "version")) ++ if (!aa_unpack_u32(e, &version, "version")) + goto fail_reset; - if (unpack_array(e, NULL, &size) != TRI_TRUE) ++ if (!aa_unpack_array(e, NULL, &size)) + goto fail_reset; + *perms = kcalloc(size, sizeof(struct aa_perms), GFP_KERNEL); + if (!*perms) + goto fail_reset; + for (i = 0; i < size; i++) { + if (!unpack_perm(e, version, &(*perms)[i])) + goto fail; + } - if (!unpack_nameX(e, AA_ARRAYEND, NULL)) ++ if (!aa_unpack_nameX(e, AA_ARRAYEND, NULL)) + goto fail; - if (!unpack_nameX(e, AA_STRUCTEND, NULL)) ++ if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL)) + goto fail; + } else + *perms = NULL; + + return size; + + fail: + kfree(*perms); + fail_reset: + e->pos = pos; + return -EPROTO; + } + + static int unpack_pdb(struct aa_ext *e, struct aa_policydb *policy, + bool required_dfa, bool required_trans, + const char **info) + { + void *pos = e->pos; + int i, flags, error = -EPROTO; + ssize_t size; + + size = unpack_perms_table(e, &policy->perms); + if (size < 0) { + error = size; + policy->perms = NULL; + *info = "failed to unpack - perms"; + goto fail; + } + policy->size = size; + + if (policy->perms) { + /* perms table present accept is index */ + flags = TO_ACCEPT1_FLAG(YYTD_DATA32); + } else { + /* packed perms in accept1 and accept2 */ + flags = TO_ACCEPT1_FLAG(YYTD_DATA32) | + TO_ACCEPT2_FLAG(YYTD_DATA32); + } + + policy->dfa = unpack_dfa(e, flags); + if (IS_ERR(policy->dfa)) { + error = PTR_ERR(policy->dfa); + policy->dfa = NULL; + *info = "failed to unpack - dfa"; + goto fail; + } else if (!policy->dfa) { + if (required_dfa) { + *info = "missing required dfa"; + goto fail; + } + goto out; + } + + /* + * only unpack the following if a dfa is present + * + * sadly start was given different names for file and policydb + * but since it is optional we can try both + */ - if (!unpack_u32(e, &policy->start[0], "start")) ++ if (!aa_unpack_u32(e, &policy->start[0], "start")) + /* default start state */ + policy->start[0] = DFA_START; - if (!unpack_u32(e, &policy->start[AA_CLASS_FILE], "dfa_start")) { ++ if (!aa_unpack_u32(e, &policy->start[AA_CLASS_FILE], "dfa_start")) { + /* default start state for xmatch and file dfa */ + policy->start[AA_CLASS_FILE] = DFA_START; + } /* setup class index */ + for (i = AA_CLASS_FILE + 1; i <= AA_CLASS_LAST; i++) { + policy->start[i] = aa_dfa_next(policy->dfa, policy->start[0], + i); + } + if (!unpack_trans_table(e, &policy->trans) && required_trans) { + *info = "failed to unpack profile transition table"; + goto fail; + } + + /* TODO: move compat mapping here, requires dfa merging first */ + /* TODO: move verify here, it has to be done after compat mappings */ + out: + return 0; + + fail: + e->pos = pos; + return error; + } + static u32 strhash(const void *data, u32 len, u32 seed) { const char * const *key = data; @@@ -683,26 -858,29 +821,29 @@@ static struct aa_profile *unpack_profil } profile = aa_alloc_profile(name, NULL, GFP_KERNEL); - if (!profile) - return ERR_PTR(-ENOMEM); + if (!profile) { + info = "out of memory"; + error = -ENOMEM; + goto fail; + } + rules = list_first_entry(&profile->rules, typeof(*rules), list); /* profile renaming is optional */ - (void) unpack_str(e, &profile->rename, "rename"); + (void) aa_unpack_str(e, &profile->rename, "rename"); /* attachment string is optional */ - (void) aa_unpack_str(e, &profile->attach, "attach"); - (void) unpack_str(e, &profile->attach.xmatch_str, "attach"); ++ (void) aa_unpack_str(e, &profile->attach.xmatch_str, "attach"); /* xmatch is optional and may be NULL */ - profile->xmatch = unpack_dfa(e); - if (IS_ERR(profile->xmatch)) { - error = PTR_ERR(profile->xmatch); - profile->xmatch = NULL; + error = unpack_pdb(e, &profile->attach.xmatch, false, false, &info); + if (error) { info = "bad xmatch"; goto fail; } - /* xmatch_len is not optional if xmatch is set */ - if (profile->xmatch) { + + /* neither xmatch_len not xmatch_perms are optional if xmatch is set */ + if (profile->attach.xmatch.dfa) { - if (!unpack_u32(e, &tmp, NULL)) { + if (!aa_unpack_u32(e, &tmp, NULL)) { info = "missing xmatch len"; goto fail; } @@@ -757,38 -943,38 +906,38 @@@ profile->path_flags = PATH_MEDIATE_DELETED; info = "failed to unpack profile capabilities"; - if (!aa_unpack_u32(e, &(profile->caps.allow.cap[0]), NULL)) - if (!unpack_u32(e, &(rules->caps.allow.cap[0]), NULL)) ++ if (!aa_unpack_u32(e, &(rules->caps.allow.cap[0]), NULL)) goto fail; - if (!aa_unpack_u32(e, &(profile->caps.audit.cap[0]), NULL)) - if (!unpack_u32(e, &(rules->caps.audit.cap[0]), NULL)) ++ if (!aa_unpack_u32(e, &(rules->caps.audit.cap[0]), NULL)) goto fail; - if (!aa_unpack_u32(e, &(profile->caps.quiet.cap[0]), NULL)) - if (!unpack_u32(e, &(rules->caps.quiet.cap[0]), NULL)) ++ if (!aa_unpack_u32(e, &(rules->caps.quiet.cap[0]), NULL)) goto fail; - if (!unpack_u32(e, &tmpcap.cap[0], NULL)) + if (!aa_unpack_u32(e, &tmpcap.cap[0], NULL)) goto fail; info = "failed to unpack upper profile capabilities"; - if (unpack_nameX(e, AA_STRUCT, "caps64")) { + if (aa_unpack_nameX(e, AA_STRUCT, "caps64")) { /* optional upper half of 64 bit caps */ - if (!aa_unpack_u32(e, &(profile->caps.allow.cap[1]), NULL)) - if (!unpack_u32(e, &(rules->caps.allow.cap[1]), NULL)) ++ if (!aa_unpack_u32(e, &(rules->caps.allow.cap[1]), NULL)) goto fail; - if (!aa_unpack_u32(e, &(profile->caps.audit.cap[1]), NULL)) - if (!unpack_u32(e, &(rules->caps.audit.cap[1]), NULL)) ++ if (!aa_unpack_u32(e, &(rules->caps.audit.cap[1]), NULL)) goto fail; - if (!aa_unpack_u32(e, &(profile->caps.quiet.cap[1]), NULL)) - if (!unpack_u32(e, &(rules->caps.quiet.cap[1]), NULL)) ++ if (!aa_unpack_u32(e, &(rules->caps.quiet.cap[1]), NULL)) goto fail; - if (!unpack_u32(e, &(tmpcap.cap[1]), NULL)) + if (!aa_unpack_u32(e, &(tmpcap.cap[1]), NULL)) goto fail; - if (!unpack_nameX(e, AA_STRUCTEND, NULL)) + if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL)) goto fail; } info = "failed to unpack extended profile capabilities"; - if (unpack_nameX(e, AA_STRUCT, "capsx")) { + if (aa_unpack_nameX(e, AA_STRUCT, "capsx")) { /* optional extended caps mediation mask */ - if (!aa_unpack_u32(e, &(profile->caps.extended.cap[0]), NULL)) - if (!unpack_u32(e, &(rules->caps.extended.cap[0]), NULL)) ++ if (!aa_unpack_u32(e, &(rules->caps.extended.cap[0]), NULL)) goto fail; - if (!aa_unpack_u32(e, &(profile->caps.extended.cap[1]), NULL)) - if (!unpack_u32(e, &(rules->caps.extended.cap[1]), NULL)) ++ if (!aa_unpack_u32(e, &(rules->caps.extended.cap[1]), NULL)) goto fail; - if (!unpack_nameX(e, AA_STRUCTEND, NULL)) + if (!aa_unpack_nameX(e, AA_STRUCTEND, NULL)) goto fail; } @@@ -807,62 -993,55 +956,55 @@@ goto fail; } - if (unpack_nameX(e, AA_STRUCT, "policydb")) { + if (aa_unpack_nameX(e, AA_STRUCT, "policydb")) { /* generic policy dfa - optional and may be NULL */ info = "failed to unpack policydb"; - profile->policy.dfa = unpack_dfa(e); - if (IS_ERR(profile->policy.dfa)) { - error = PTR_ERR(profile->policy.dfa); - profile->policy.dfa = NULL; - goto fail; - } else if (!profile->policy.dfa) { - error = -EPROTO; + error = unpack_pdb(e, &rules->policy, true, false, + &info); + if (error) goto fail; - } - if (!aa_unpack_u32(e, &profile->policy.start[0], "start")) - /* default start state */ - profile->policy.start[0] = DFA_START; - /* setup class index */ - for (i = AA_CLASS_FILE; i <= AA_CLASS_LAST; i++) { - profile->policy.start[i] = - aa_dfa_next(profile->policy.dfa, - profile->policy.start[0], - i); - } + /* Fixup: drop when we get rid of start array */ + if (aa_dfa_next(rules->policy.dfa, rules->policy.start[0], + AA_CLASS_FILE)) + rules->policy.start[AA_CLASS_FILE] = + aa_dfa_next(rules->policy.dfa, + rules->policy.start[0], + AA_CLASS_FILE); - if (!unpack_nameX(e, AA_STRUCTEND, NULL)) + 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; + } } else - profile->policy.dfa = aa_get_dfa(nulldfa); + rules->policy.dfa = aa_get_dfa(nulldfa); /* get file rules */ - profile->file.dfa = unpack_dfa(e); - if (IS_ERR(profile->file.dfa)) { - error = PTR_ERR(profile->file.dfa); - profile->file.dfa = NULL; - info = "failed to unpack profile file rules"; + error = unpack_pdb(e, &rules->file, false, true, &info); + if (error) { goto fail; - } else if (profile->file.dfa) { - if (!aa_unpack_u32(e, &profile->file.start, "dfa_start")) - /* default start state */ - profile->file.start = DFA_START; - } else if (profile->policy.dfa && - profile->policy.start[AA_CLASS_FILE]) { - profile->file.dfa = aa_get_dfa(profile->policy.dfa); - profile->file.start = profile->policy.start[AA_CLASS_FILE]; + } else if (rules->file.dfa) { + 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 - profile->file.dfa = aa_get_dfa(nulldfa); - - if (!unpack_trans_table(e, profile)) { - info = "failed to unpack profile transition table"; - goto fail; - } + rules->file.dfa = aa_get_dfa(nulldfa); + error = -EPROTO; - if (unpack_nameX(e, AA_STRUCT, "data")) { + if (aa_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); diff --cc security/apparmor/policy_unpack_test.c index f25cf2a023d5,7465da42492d..e1bfdab524b7 --- a/security/apparmor/policy_unpack_test.c +++ b/security/apparmor/policy_unpack_test.c @@@ -147,8 -144,8 +147,7 @@@ static void policy_unpack_test_unpack_a puf->e->pos += TEST_ARRAY_BUF_OFFSET; - array_size = aa_unpack_array(puf->e, NULL); - - KUNIT_EXPECT_EQ(test, unpack_array(puf->e, NULL, &array_size), - TRI_TRUE); ++ KUNIT_EXPECT_TRUE(test, aa_unpack_array(puf->e, NULL, &array_size)); KUNIT_EXPECT_EQ(test, array_size, (u16)TEST_ARRAY_SIZE); KUNIT_EXPECT_PTR_EQ(test, puf->e->pos, puf->e->start + TEST_ARRAY_BUF_OFFSET + sizeof(u16) + 1); @@@ -162,8 -159,8 +161,7 @@@ static void policy_unpack_test_unpack_a puf->e->pos += TEST_NAMED_ARRAY_BUF_OFFSET; - array_size = aa_unpack_array(puf->e, name); - - KUNIT_EXPECT_EQ(test, unpack_array(puf->e, name, &array_size), - TRI_TRUE); ++ KUNIT_EXPECT_TRUE(test, aa_unpack_array(puf->e, name, &array_size)); KUNIT_EXPECT_EQ(test, array_size, (u16)TEST_ARRAY_SIZE); KUNIT_EXPECT_PTR_EQ(test, puf->e->pos, puf->e->start + TEST_ARRAY_BUF_OFFSET + sizeof(u16) + 1); @@@ -178,9 -175,8 +176,7 @@@ static void policy_unpack_test_unpack_a puf->e->pos += TEST_NAMED_ARRAY_BUF_OFFSET; puf->e->end = puf->e->start + TEST_ARRAY_BUF_OFFSET + sizeof(u16); - array_size = aa_unpack_array(puf->e, name); - - KUNIT_EXPECT_EQ(test, array_size, 0); - KUNIT_EXPECT_EQ(test, unpack_array(puf->e, name, &array_size), - TRI_FALSE); ++ KUNIT_EXPECT_FALSE(test, aa_unpack_array(puf->e, name, &array_size)); KUNIT_EXPECT_PTR_EQ(test, puf->e->pos, puf->e->start + TEST_NAMED_ARRAY_BUF_OFFSET); }