Merge tag 'apparmor-pr-2024-07-25' of git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor

Pull apparmor updates from John Johansen: "Cleanups - optimization: try to avoid refing the label in apparmor_file_open - remove useless static inline function is_deleted - use kvfree_sensitive to free data->data - fix typo in kernel doc Bug fixes: - unpack transition table if dfa is not present - test: add MODULE_DESCRIPTION() - take nosymfollow flag into account - fix possible NULL pointer dereference - fix null pointer deref when receiving skb during sock creation" * tag 'apparmor-pr-2024-07-25' of git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor: apparmor: unpack transition table if dfa is not present apparmor: try to avoid refing the label in apparmor_file_open apparmor: test: add MODULE_DESCRIPTION() apparmor: take nosymfollow flag into account apparmor: fix possible NULL pointer dereference apparmor: fix typo in kernel doc apparmor: remove useless static inline function is_deleted apparmor: use kvfree_sensitive to free data->data apparmor: Fix null pointer deref when receiving skb during sock creation

Author: torvalds

security/apparmor/apparmorfs.c

@@ -1692,6 +1692,10 @@ int __aafs_profile_mkdir(struct aa_profile *profile, struct dentry *parent)
 struct aa_profile *p;
 p = aa_deref_parent(profile);
 dent = prof_dir(p);
+if (!dent) {
+error = -ENOENT;
+goto fail2;
+}
 /* adding to parent that previously didn't have children */
 dent = aafs_create_dir("profiles", dent);
 if (IS_ERR(dent))

security/apparmor/file.c

@@ -144,19 +144,6 @@ int aa_audit_file(const struct cred *subj_cred,
 return aa_audit(type, profile, &ad, file_audit_cb);
 }
 
-/**
- * is_deleted - test if a file has been completely unlinked
- * @dentry: dentry of file to test for deletion (NOT NULL)
- *
- * Returns: true if deleted else false
- */
-static inline bool is_deleted(struct dentry *dentry)
-{
-if (d_unlinked(dentry) && d_backing_inode(dentry)->i_nlink == 0)
-return true;
-return false;
-}
-
 static int path_name(const char *op, const struct cred *subj_cred,
  struct aa_label *label,
  const struct path *path, int flags, char *buffer,

security/apparmor/include/cred.h

@@ -63,6 +63,26 @@ static inline struct aa_label *aa_get_newest_cred_label(const struct cred *cred)
 return aa_get_newest_label(aa_cred_raw_label(cred));
 }
 
+static inline struct aa_label *aa_get_newest_cred_label_condref(const struct cred *cred,
+bool *needput)
+{
+struct aa_label *l = aa_cred_raw_label(cred);
+
+if (unlikely(label_is_stale(l))) {
+*needput = true;
+return aa_get_newest_label(l);
+}
+
+*needput = false;
+return l;
+}
+
+static inline void aa_put_label_condref(struct aa_label *l, bool needput)
+{
+if (unlikely(needput))
+aa_put_label(l);
+}
+
 /**
  * aa_current_raw_label - find the current tasks confining label
  *

security/apparmor/lsm.c

@@ -461,6 +461,7 @@ static int apparmor_file_open(struct file *file)
 struct aa_file_ctx *fctx = file_ctx(file);
 struct aa_label *label;
 int error = 0;
+bool needput;
 
 if (!path_mediated_fs(file->f_path.dentry))
 return 0;
@@ -477,7 +478,7 @@ static int apparmor_file_open(struct file *file)
 return 0;
 }
 
-label = aa_get_newest_cred_label(file->f_cred);
+label = aa_get_newest_cred_label_condref(file->f_cred, &needput);
 if (!unconfined(label)) {
 struct mnt_idmap *idmap = file_mnt_idmap(file);
 struct inode *inode = file_inode(file);
@@ -494,7 +495,7 @@ static int apparmor_file_open(struct file *file)
 /* todo cache full allowed permissions set and state */
 fctx->allow = aa_map_file_to_perms(file);
 }
-aa_put_label(label);
+aa_put_label_condref(label, needput);
 
 return error;
 }
@@ -1124,7 +1125,7 @@ static int apparmor_socket_create(int family, int type, int protocol, int kern)
  * @sock: socket that is being setup
  * @family: family of socket being created
  * @type: type of the socket
- * @ptotocol: protocol of the socket
+ * @protocol: protocol of the socket
  * @kern: socket is a special kernel socket
  *
  * Note:
@@ -1304,6 +1305,13 @@ static int apparmor_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
 if (!skb->secmark)
 return 0;
 
+/*
+ * If reach here before socket_post_create hook is called, in which
+ * case label is null, drop the packet.
+ */
+if (!ctx->label)
+return -EACCES;
+
 return apparmor_secmark_check(ctx->label, OP_RECVMSG, AA_MAY_RECEIVE,
  skb->secmark, sk);
 }

security/apparmor/mount.c

@@ -44,6 +44,8 @@ static void audit_mnt_flags(struct audit_buffer *ab, unsigned long flags)
 audit_log_format(ab, ", mand");
 if (flags & MS_DIRSYNC)
 audit_log_format(ab, ", dirsync");
+if (flags & MS_NOSYMFOLLOW)
+audit_log_format(ab, ", nosymfollow");
 if (flags & MS_NOATIME)
 audit_log_format(ab, ", noatime");
 if (flags & MS_NODIRATIME)

security/apparmor/policy.c

@@ -225,7 +225,7 @@ static void aa_free_data(void *ptr, void *arg)
 {
 struct aa_data *data = ptr;
 
-kfree_sensitive(data->data);
+kvfree_sensitive(data->data, data->size);
 kfree_sensitive(data->key);
 kfree_sensitive(data);
 }

security/apparmor/policy_unpack.c

@@ -747,34 +747,42 @@ static int unpack_pdb(struct aa_ext *e, struct aa_policydb **policy,
 *info = "missing required dfa";
 goto fail;
 }
-goto out;
+} else {
+/*
+ * 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 (!aa_unpack_u32(e, &pdb->start[0], "start"))
+/* default start state */
+pdb->start[0] = DFA_START;
+if (!aa_unpack_u32(e, &pdb->start[AA_CLASS_FILE], "dfa_start")) {
+/* default start state for xmatch and file dfa */
+pdb->start[AA_CLASS_FILE] = DFA_START;
+}/* setup class index */
+for (i = AA_CLASS_FILE + 1; i <= AA_CLASS_LAST; i++) {
+pdb->start[i] = aa_dfa_next(pdb->dfa, pdb->start[0],
+ i);
+}
 }
 
 /*
- * 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
+ * Unfortunately due to a bug in earlier userspaces, a
+ * transition table may be present even when the dfa is
+ * not. For compatibility reasons unpack and discard.
  */
-if (!aa_unpack_u32(e, &pdb->start[0], "start"))
-/* default start state */
-pdb->start[0] = DFA_START;
-if (!aa_unpack_u32(e, &pdb->start[AA_CLASS_FILE], "dfa_start")) {
-/* default start state for xmatch and file dfa */
-pdb->start[AA_CLASS_FILE] = DFA_START;
-}/* setup class index */
-for (i = AA_CLASS_FILE + 1; i <= AA_CLASS_LAST; i++) {
-pdb->start[i] = aa_dfa_next(pdb->dfa, pdb->start[0],
- i);
-}
 if (!unpack_trans_table(e, &pdb->trans) && required_trans) {
 *info = "failed to unpack profile transition table";
 goto fail;
 }
 
+if (!pdb->dfa && pdb->trans.table)
+aa_free_str_table(&pdb->trans);
+
 /* TODO: move compat mapping here, requires dfa merging first */
 /* TODO: move verify here, it has to be done after compat mappings */
-out:
+
 *policy = pdb;
 return 0;
 
@@ -1071,6 +1079,7 @@ static struct aa_profile *unpack_profile(struct aa_ext *e, char **ns_name)
 
 if (rhashtable_insert_fast(profile->data, &data->head,
  profile->data->p)) {
+kvfree_sensitive(data->data, data->size);
 kfree_sensitive(data->key);
 kfree_sensitive(data);
 info = "failed to insert data to table";

security/apparmor/policy_unpack_test.c

@@ -604,4 +604,5 @@ static struct kunit_suite apparmor_policy_unpack_test_module = {
 
 kunit_test_suite(apparmor_policy_unpack_test_module);
 
+MODULE_DESCRIPTION("KUnit tests for AppArmor's policy unpack");
 MODULE_LICENSE("GPL");