[Devel] [PATCH vz9] cgroup: close a race window on cgrp removal

[Konstantin Khorenko]

Kirill, please review the patch.

--
Best regards,

Konstantin Khorenko,
Virtuozzo Linux Kernel Team

On 15.12.2021 13:24, Nikita Yushchenko wrote:
> Cgroup removal via rmdir() can race with preparation to call release
> agent for the same cgroup (triggered e.g. by a recent exit of the last
> cgroup's task). Although quite unlikely, that can result into cgrp
> object reaching kfree() while it is still ibeing used by
> cgroup1_release_agent().
> 
> This scenario is a possible reason for use-after-free caught by KASAN:
> 
>      Dec 10 01:50:19 sqvm2-0806 kernel: BUG: KASAN: use-after-free in cgroup_path_ns_locked+0xcd/0x110
>      Dec 10 01:50:19 sqvm2-0806 kernel: Read of size 8 at addr ffff888134b102b8 by task kworker/u8:0/769807
> ...
>      Dec 10 01:50:19 sqvm2-0806 kernel: dump_stack_lvl+0x56/0x7b
>      Dec 10 01:50:19 sqvm2-0806 kernel: print_address_description.constprop.8+0x21/0x150
>      Dec 10 01:50:19 sqvm2-0806 kernel: ? cgroup_path_ns_locked+0xcd/0x110
>      Dec 10 01:50:19 sqvm2-0806 kernel: ? cgroup_path_ns_locked+0xcd/0x110
>      Dec 10 01:50:19 sqvm2-0806 kernel: kasan_report.cold.14+0x7f/0x11b
>      Dec 10 01:50:19 sqvm2-0806 kernel: ? do_raw_spin_lock+0x250/0x290
>      Dec 10 01:50:19 sqvm2-0806 kernel: ? cgroup_path_ns_locked+0xcd/0x110
>      Dec 10 01:50:19 sqvm2-0806 kernel: cgroup_path_ns_locked+0xcd/0x110
>      Dec 10 01:50:19 sqvm2-0806 kernel: cgroup_path_ns+0x42/0x70
>      Dec 10 01:50:19 sqvm2-0806 kernel: cgroup1_release_agent+0x27e/0x680
> ...
> 
> Close the race window by taking an extra reference to cgrp for the time
> while cgroup1_release_agent() uses it.
> 
> https://jira.sw.ru/browse/PSBM-136884
> Signed-off-by: Nikita Yushchenko <nikita.yushchenko at virtuozzo.com>
> ---
>   kernel/cgroup/cgroup-v1.c | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
> 
> diff --git a/kernel/cgroup/cgroup-v1.c b/kernel/cgroup/cgroup-v1.c
> index d58faf071e2c..3a59357b7989 100644
> --- a/kernel/cgroup/cgroup-v1.c
> +++ b/kernel/cgroup/cgroup-v1.c
> @@ -875,6 +875,18 @@ void cgroup1_release_agent(struct work_struct *work)
>   				  struct cgroup,
>   				  release_list);
>   		list_del_init(&cgrp->release_list);
> +
> +		/*
> +		 * Once the lock gets released, concurrently running removal
> +		 * of the same cgrp via rmdir() won't find the cgrp in
> +		 * ve_rm_from_release_list() called from cgroup_release_locked()
> +		 * and can progress up to end and put the last reference to
> +		 * cgrp. This can result into cgrp reaching kfree() before this
> +		 * thread stops using it.
> +		 */
> +		if (WARN_ON(!cgroup_tryget(cgrp)))
> +			continue;
> +
>   		spin_unlock_irqrestore(&ve->release_list_lock, flags);
>   
>   		rcu_read_lock();
> @@ -907,6 +919,7 @@ void cgroup1_release_agent(struct work_struct *work)
>   					    "%s %s failed: %d\n",
>   					    agentbuf, pathbuf, ret);
>   continue_locked:
> +		cgroup_put(cgrp);
>   		spin_lock_irqsave(&ve->release_list_lock, flags);
>   	}
>   	spin_unlock_irqrestore(&ve->release_list_lock, flags);
>