netdev - cgroup pointed by sock is leaked on mode switch

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-next>] [day] [month] [year] [list]

Date:   Sat, 2 May 2020 18:27:21 +0800
From:   Yang Yingliang <yangyingliang@...wei.com>
To:     <tj@...nel.org>
CC:     <linux-kernel@...r.kernel.org>, <cgroups@...r.kernel.org>,
        <netdev@...r.kernel.org>,
        "Libin (Huawei)" <huawei.libin@...wei.com>,
        <yangyingliang@...wei.com>, <guofan5@...wei.com>,
        <wangkefeng.wang@...wei.com>
Subject: cgroup pointed by sock is leaked on mode switch

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