Re: [RFC PATCH bpf-next 1/8] cgroup: Don't have to hold cgroup_mutex in task_cgroup_from_root()

Yafang Shao
2023-10-10T04:05:53Z

On Mon, Oct 9, 2023 at 10:45 PM Michal Koutný <mkoutny@suse.com> wrote:
>
> On Sat, Oct 07, 2023 at 02:02:57PM +0000, Yafang Shao <laoar.shao@gmail.com> wrote:
> > The task cannot modify cgroups if we have already acquired the
> > css_set_lock, thus eliminating the need to hold the cgroup_mutex. Following
> > this change, task_cgroup_from_root() can be employed in non-sleepable contexts.
>
> IIRC, cset_cgroup_from_root() needs cgroup_mutex to make sure the `root`
> doesn't disappear under hands (synchronization with
> cgroup_destroy_root().

current_cgns_cgroup_from_root() doesn't hold the cgroup_mutext as
well. Could this potentially lead to issues, such as triggering the
BUG_ON() in __cset_cgroup_from_root(), if the root has already been
destroyed?

> However, as I look at it now, cgroup_mutex won't synchronize against
> cgroup_kill_sb(), it may worked by accident(?) nowadays (i.e. callers
> pinned the root implicitly in another way).
>
> Still, it'd be good to have an annotation that ensures, the root is around
> when using it. (RCU read lock might be fine but you'd need
> cgroup_get_live() if passing it out of the RCU read section.)
>
> Basically, the code must be made safe against cgroup v1 unmounts.

What we aim to protect against is changes to the `root_list`, which
occur exclusively during cgroup setup and destroy paths. Would it be
beneficial to introduce a dedicated root_list_lock specifically for
this purpose? This approach could potentially reduce the need for the
broader cgroup_mutex in other scenarios.

--
Regards
Yafang