On Sun, Oct 9, 2022 at 6:10 AM Christian A. Ehrhardt <lk@xxxxxxx> wrote: > > > Since commit f3a2aebdd6, Version 1 cgroups no longer cause an > error when used with CLONE_INTO_CGROUP. However, the permission > checks performed during clone assume a Version 2 cgroup. > > Restore the error check for V1 cgroups in the clone() path. > > Reported-by: syzbot+534ee3d24c37c411f37f@xxxxxxxxxxxxxxxxxxxxxxxxx > Link: https://lore.kernel.org/lkml/000000000000385cbf05ea3f1862@xxxxxxxxxx/ > Fixes: f3a2aebdd6 ("cgroup: enable cgroup_get_from_file() on cgroup1") Thanks for fixing this, and sorry if this caused a mess. cgroup_get_from_file() independently seemed like it can support cgroup1, I didn't realize that some of the callers depend on the fact that it only supports cgroup2. +Andrii Nakryiko +Alexei Starovoitov +Martin KaFai Lau +bpf I wonder if BPF users have this dependency. Does cgroup_bpf_attach() also depend on cgroup_get_from_fd() (which calls cgroup_get_from_file()) eliminating v1 cgroups? It seems like cgroup storages (and some other places) use cgroup ids. Collisions can happen in cgroup1 ids so I am assuming we want to add a check there as well. Perhaps in cgroup_bpf_attach() ? I can send a patch for this if that's the case. > Signed-off-by: Christian A. Ehrhardt <lk@xxxxxxx> > --- > kernel/cgroup/cgroup.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c > index b6e3110b3ea7..f7fc3afa88c1 100644 > --- a/kernel/cgroup/cgroup.c > +++ b/kernel/cgroup/cgroup.c > @@ -6244,6 +6244,11 @@ static int cgroup_css_set_fork(struct kernel_clone_args *kargs) > goto err; > } > > + if (!cgroup_on_dfl(dst_cgrp)) { > + ret = -EBADF; > + goto err; > + } > + > if (cgroup_is_dead(dst_cgrp)) { > ret = -ENODEV; > goto err; > -- > 2.34.1 >
