Merge tag 'apparmor-pr-2023-07-06' of git://git.kernel.org/pub/scm/linux/kernel/git...
authorLinus Torvalds <torvalds@linux-foundation.org>
Fri, 7 Jul 2023 16:55:31 +0000 (09:55 -0700)
committerLinus Torvalds <torvalds@linux-foundation.org>
Fri, 7 Jul 2023 16:55:31 +0000 (09:55 -0700)
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

security/apparmor/crypto.c
security/apparmor/file.c
security/apparmor/lsm.c
security/apparmor/policy.c
security/apparmor/policy_compat.c
security/apparmor/policy_unpack.c
security/apparmor/policy_unpack_test.c
security/apparmor/secid.c

index b498ed3..6724e2f 100644 (file)
@@ -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;
 
index 9119ddd..698b124 100644 (file)
@@ -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)
 {
index f431251..c9463bd 100644 (file)
@@ -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;
 }
index 51e8184..b38f7b2 100644 (file)
@@ -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.
index cc89d1e..0cb02da 100644 (file)
@@ -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;
 
index cf2ceec..694fb7a 100644 (file)
@@ -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;
index e1bfdab..5c9bde2 100644 (file)
@@ -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;
 
index 24a0e23..83d3d1e 100644 (file)
@@ -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)