Re: [PATCH v4 3/3] AppArmor: add support for lsm_config_self_policy and lsm_config_system_policy

On 2025/07/01 18:17, Maxime Bélair wrote:

+static int apparmor_lsm_config_self_policy(u32 lsm_id, u32 op, void __user *buf,
+				      size_t size, u32 flags)
+{
+	char *name;
+	long name_size;
+	int ret;
+
+	if (op != LSM_POLICY_LOAD || flags)
+		return -EOPNOTSUPP;
+	if (size > AA_PROFILE_NAME_MAX_SIZE)
+		return -E2BIG;
+
+	name = kmalloc(size, GFP_KERNEL);
+	if (!name)
+		return -ENOMEM;
+
+
+	name_size = strncpy_from_user(name, buf, size);
+	if (name_size < 0) {
+		kfree(name);
+		return name_size;
+	}
+
+	ret = aa_change_profile(name, AA_CHANGE_STACK);

If size == 0, name == ZERO_SIZE_PTR and name_size == 0.
Then, aa_change_profile() will oops due to ZERO_SIZE_PTR deref.

+
+	kfree(name);
+
+	return ret;
+}
+
+/**
+ * apparmor_lsm_config_system_policy - Load or replace a system policy
+ * @lsm_id: AppArmor ID (LSM_ID_APPARMOR). Unused here
+ * @op: operation to perform. Currently, only LSM_POLICY_LOAD is supported
+ * @buf: user-supplied buffer in the form "<ns>\0<policy>"
+ *        <ns> is the namespace to load the policy into (empty string for root)
+ *        <policy> is the policy to load
+ * @size: size of @buf
+ * @flags: reserved for future uses; must be zero
+ *
+ * Returns: 0 on success, negative value on error
+ */
+static int apparmor_lsm_config_system_policy(u32 lsm_id, u32 op, void __user *buf,
+				      size_t size, u32 flags)
+{
+	loff_t pos = 0; // Partial writing is not currently supported
+	char name[AA_PROFILE_NAME_MAX_SIZE];
+	long name_size;
+
+	if (op != LSM_POLICY_LOAD || flags)
+		return -EOPNOTSUPP;
+	if (size > AA_PROFILE_MAX_SIZE)
+		return -E2BIG;
+
+	name_size = strncpy_from_user(name, buf, AA_PROFILE_NAME_MAX_SIZE);
+	if (name_size < 0)
+		return name_size;
+	else if (name_size == AA_PROFILE_NAME_MAX_SIZE)
+		return -E2BIG;
+
+	return aa_profile_load_ns_name(name, name_size, buf + name_size + 1,
+				       size - name_size - 1, &pos);

If size == 0 and *name == '\0', name_size == 0. Then, size will be -1 at aa_profile_load_ns_name()
and WARN_ON_ONCE() in __kvmalloc_node_noprof() from kvzalloc() from policy_update() will trigger?

You need more stricter checks to verify that @buf is in the form "<ns>\0<policy>".
strncpy_from_user() should not try to read more than @size bytes.

+}
+
+

Also, in [PATCH v4 2/3], why do you need lines below?
These functions are supposed to be called via only syscalls, aren't these?

+EXPORT_SYMBOL(security_lsm_config_self_policy);
+EXPORT_SYMBOL(security_lsm_config_system_policy);