lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 24 Jun 2024 10:59:16 +0800
From: chenridong <chenridong@...wei.com>
To: Waiman Long <longman@...hat.com>, <tj@...nel.org>,
	<lizefan.x@...edance.com>, <hannes@...xchg.org>
CC: <bpf@...r.kernel.org>, <cgroups@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH -next] cgroup: fix uaf when proc_cpuset_show


On 2024/6/22 23:05, Waiman Long wrote:
>
> On 6/22/24 07:38, Chen Ridong wrote:
>> We found a refcount UAF bug as follows:
>>
>> BUG: KASAN: use-after-free in cgroup_path_ns+0x112/0x150
>> Read of size 8 at addr ffff8882a4b242b8 by task atop/19903
>>
>> CPU: 27 PID: 19903 Comm: atop Kdump: loaded Tainted: GF
>> Call Trace:
>>   dump_stack+0x7d/0xa7
>>   print_address_description.constprop.0+0x19/0x170
>>   ? cgroup_path_ns+0x112/0x150
>>   __kasan_report.cold+0x6c/0x84
>>   ? print_unreferenced+0x390/0x3b0
>>   ? cgroup_path_ns+0x112/0x150
>>   kasan_report+0x3a/0x50
>>   cgroup_path_ns+0x112/0x150
>>   proc_cpuset_show+0x164/0x530
>>   proc_single_show+0x10f/0x1c0
>>   seq_read_iter+0x405/0x1020
>>   ? aa_path_link+0x2e0/0x2e0
>>   seq_read+0x324/0x500
>>   ? seq_read_iter+0x1020/0x1020
>>   ? common_file_perm+0x2a1/0x4a0
>>   ? fsnotify_unmount_inodes+0x380/0x380
>>   ? bpf_lsm_file_permission_wrapper+0xa/0x30
>>   ? security_file_permission+0x53/0x460
>>   vfs_read+0x122/0x420
>>   ksys_read+0xed/0x1c0
>>   ? __ia32_sys_pwrite64+0x1e0/0x1e0
>>   ? __audit_syscall_exit+0x741/0xa70
>>   do_syscall_64+0x33/0x40
>>   entry_SYSCALL_64_after_hwframe+0x67/0xcc
>>
>> This is also reported by: 
>> https://syzkaller.appspot.com/bug?extid=9b1ff7be974a403aa4cd
>>
>> This can be reproduced by the following methods:
>> 1.add an mdelay(1000) before acquiring the cgroup_lock In the
>>   cgroup_path_ns function.
>> 2.$cat /proc/<pid>/cpuset   repeatly.
>> 3.$mount -t cgroup -o cpuset cpuset /sys/fs/cgroup/cpuset/
>> $umount /sys/fs/cgroup/cpuset/   repeatly.
>>
>> The race that cause this bug can be shown as below:
>>
>> (umount)        |    (cat /proc/<pid>/cpuset)
>> css_release        |    proc_cpuset_show
>> css_release_work_fn    |    css = task_get_css(tsk, cpuset_cgrp_id);
>> css_free_rwork_fn    |    cgroup_path_ns(css->cgroup, ...);
>> cgroup_destroy_root    |    mutex_lock(&cgroup_mutex);
>> rebind_subsystems    |
>> cgroup_free_root     |
>>             |    // cgrp was freed, UAF
>>             |    cgroup_path_ns_locked(cgrp,..);
>>
>> When the cpuset is initialized, the root node top_cpuset.css.cgrp
>> will point to &cgrp_dfl_root.cgrp. In cgroup v1, the mount operation 
>> will
>> allocate cgroup_root, and top_cpuset.css.cgrp will point to the 
>> allocated
>> &cgroup_root.cgrp. When the umount operation is executed,
>> top_cpuset.css.cgrp will be rebound to &cgrp_dfl_root.cgrp.
>>
>> The problem is that when rebinding to cgrp_dfl_root, there are cases
>> where the cgroup_root allocated by setting up the root for cgroup v1
>> is cached. This could lead to a Use-After-Free (UAF) if it is
>> subsequently freed. The descendant cgroups of cgroup v1 can only be
>> freed after the css is released. However, the css of the root will never
>> be released, yet the cgroup_root should be freed when it is unmounted.
>> This means that obtaining a reference to the css of the root does
>> not guarantee that css.cgrp->root will not be freed.
>>
>> To solve this issue, we have added a cgroup reference count in
>> the proc_cpuset_show function to ensure that css.cgrp->root will not
>> be freed prematurely. This is a temporary solution. Let's see if anyone
>> has a better solution.
>>
>> Signed-off-by: Chen Ridong <chenridong@...wei.com>
>> ---
>>   kernel/cgroup/cpuset.c | 20 ++++++++++++++++++++
>>   1 file changed, 20 insertions(+)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index c12b9fdb22a4..782eaf807173 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -5045,6 +5045,7 @@ int proc_cpuset_show(struct seq_file *m, struct 
>> pid_namespace *ns,
>>       char *buf;
>>       struct cgroup_subsys_state *css;
>>       int retval;
>> +    struct cgroup *root_cgroup = NULL;
>>         retval = -ENOMEM;
>>       buf = kmalloc(PATH_MAX, GFP_KERNEL);
>> @@ -5052,9 +5053,28 @@ int proc_cpuset_show(struct seq_file *m, 
>> struct pid_namespace *ns,
>>           goto out;
>>         css = task_get_css(tsk, cpuset_cgrp_id);
>> +    rcu_read_lock();
>> +    /*
>> +     * When the cpuset subsystem is mounted on the legacy hierarchy,
>> +     * the top_cpuset.css->cgroup does not hold a reference count of
>> +     * cgroup_root.cgroup. This makes accessing css->cgroup very
>> +     * dangerous because when the cpuset subsystem is remounted to the
>> +     * default hierarchy, the cgroup_root.cgroup that css->cgroup 
>> points
>> +     * to will be released, leading to a UAF issue. To avoid this 
>> problem,
>> +     * get the reference count of top_cpuset.css->cgroup first.
>> +     *
>> +     * This is ugly!!
>> +     */
>> +    if (css == &top_cpuset.css) {
>> +        cgroup_get(css->cgroup);
>> +        root_cgroup = css->cgroup;
>> +    }
>> +    rcu_read_unlock();
>>       retval = cgroup_path_ns(css->cgroup, buf, PATH_MAX,
>>                   current->nsproxy->cgroup_ns);
>>       css_put(css);
>> +    if (root_cgroup)
>> +        cgroup_put(root_cgroup);
>>       if (retval == -E2BIG)
>>           retval = -ENAMETOOLONG;
>>       if (retval < 0)
>
> Thanks for reporting this UAF bug. Could you try the attached patch to 
> see if it can fix the issue?
>

+/*
+ * With a cgroup v1 mount, root_css.cgroup can be freed. We need to take a
+ * reference to it to avoid UAF as proc_cpuset_show() may access the 
content
+ * of this cgroup.
+ */
  static void cpuset_bind(struct cgroup_subsys_state *root_css)
  {
+    static struct cgroup *v1_cgroup_root;
+
      mutex_lock(&cpuset_mutex);
+    if (v1_cgroup_root) {
+        cgroup_put(v1_cgroup_root);
+        v1_cgroup_root = NULL;
+    }
      spin_lock_irq(&callback_lock);

      if (is_in_v2_mode()) {
@@ -4159,6 +4170,10 @@ static void cpuset_bind(struct 
cgroup_subsys_state *root_css)
      }

      spin_unlock_irq(&callback_lock);
+    if (!cgroup_subsys_on_dfl(cpuset_cgrp_subsys)) {
+        v1_cgroup_root = root_css->cgroup;
+        cgroup_get(v1_cgroup_root);
+    }
      mutex_unlock(&cpuset_mutex);
  }

Thanks for your suggestion. If we take a reference at rebind(call 
->bind()) function, cgroup_root allocated when setting up root for 
cgroup v1 can never be released, because the reference count will never 
be reduced to zero.

We have already tried similar methods to fix this issue, however doing 
so causes another issue as mentioned previously.


Ridong


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ