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);
