cgroup pointed by sock is leaked on mode switch

[Yang Yingliang]

Hi,

I got an oom panic because cgroup is leaked.

Here is the steps :
 - run a docker with --cap-add sys_admin parameter and the systemd 
process in the docker uses both cgroupv1 and cgroupv2
 - ssh/exit from host to docker repeately

I find the number nr_dying_descendants is increasing:
linux-dVpNUK:~ # find /sys/fs/cgroup/ -name cgroup.stat -exec grep 
'^nr_dying_descendants [^0]'Â {} +
/sys/fs/cgroup/unified/cgroup.stat:nr_dying_descendants 80
/sys/fs/cgroup/unified/system.slice/cgroup.stat:nr_dying_descendants 1
/sys/fs/cgroup/unified/system.slice/system-hostos.slice/cgroup.stat:nr_dying_descendants 
1
/sys/fs/cgroup/unified/lxc/cgroup.stat:nr_dying_descendants 79
/sys/fs/cgroup/unified/lxc/5f1fdb8c54fa40c3e599613dab6e4815058b76ebada8a27bc1fe80c0d4801764/cgroup.stat:nr_dying_descendants 
78
/sys/fs/cgroup/unified/lxc/5f1fdb8c54fa40c3e599613dab6e4815058b76ebada8a27bc1fe80c0d4801764/system.slice/cgroup.stat:nr_dying_descendants 
78


The situation is as same as the commit bd1060a1d671 ("sock, cgroup: add 
sock->sk_cgroup") describes.
"On mode switch, cgroup references which are already being pointed to by 
socks may be leaked."

Do we have a fix for this leak now ?

Or how about fix this by record the cgrp2 pointer, then put it when sk 
is freeing like this:

diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index d9bd671105e2..cbb1e76ea305 100644
--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -770,6 +770,7 @@ struct sock_cgroup_data {
Â#endif
ÂÂÂÂÂÂÂÂ u64ÂÂÂÂÂÂÂ val;
ÂÂÂÂ };
+ÂÂÂ struct cgroup *cgrpv2;
Â};

Â/*
@@ -802,6 +803,7 @@ static inline void sock_cgroup_set_prioidx(struct 
sock_cgroup_data *skcd,
ÂÂÂÂÂÂÂÂ return;

ÂÂÂÂ if (!(skcd_buf.is_data & 1)) {
+ÂÂÂÂÂÂÂ WRITE_ONCE(skcd->cgrpv2, skcd_buf.val);
ÂÂÂÂÂÂÂÂ skcd_buf.val = 0;
ÂÂÂÂÂÂÂÂ skcd_buf.is_data = 1;
ÂÂÂÂ }
@@ -819,6 +821,7 @@ static inline void sock_cgroup_set_classid(struct 
sock_cgroup_data *skcd,
ÂÂÂÂÂÂÂÂ return;

ÂÂÂÂ if (!(skcd_buf.is_data & 1)) {
+ÂÂÂÂÂÂÂ WRITE_ONCE(skcd->cgrpv2, skcd_buf.val);
ÂÂÂÂÂÂÂÂ skcd_buf.val = 0;
ÂÂÂÂÂÂÂÂ skcd_buf.is_data = 1;
ÂÂÂÂ }
diff --git a/net/core/sock.c b/net/core/sock.c
index a0dda2bf9d7c..7c761ef2d32e 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1520,6 +1520,10 @@ static void sk_prot_free(struct proto *prot, 
struct sock *sk)
ÂÂÂÂ slab = prot->slab;

ÂÂÂÂ cgroup_sk_free(&sk->sk_cgrp_data);
+ÂÂÂ if (sk->sk_cgrp_data.cgrpv2) {
+ÂÂÂÂÂÂÂ cgroup_put(sk->sk_cgrp_data.cgrpv2);
+ÂÂÂÂÂÂÂ sk->sk_cgrp_data.cgrpv2 = NULL;
+ÂÂÂ }
ÂÂÂÂ mem_cgroup_sk_free(sk);
ÂÂÂÂ security_sk_free(sk);
ÂÂÂÂ if (slab != NULL)


Thanks,
Yang