netdev - Re: [PATCH bpf-next] selftests/bpf: fix test_dev

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

Message-ID: <20180123175901.it77if7w655bwwew@ast-mbp.dhcp.thefacebook.com>
Date:   Tue, 23 Jan 2018 09:59:03 -0800
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Daniel Borkmann <daniel@...earbox.net>
Cc:     Alexei Starovoitov <ast@...nel.org>, davem@...emloft.net,
        guro@...com, tj@...nel.org, netdev@...r.kernel.org
Subject: Re: [PATCH bpf-next] selftests/bpf: fix test_dev_cgroup

On Tue, Jan 23, 2018 at 06:50:04PM +0100, Daniel Borkmann wrote:
> On 01/23/2018 05:48 AM, Alexei Starovoitov wrote:
> > The test incorrectly doing
> > mkdir /mnt/cgroup-test-work-dirtest-bpf-based-device-cgroup
> > instead of
> > mkdir /mnt/cgroup-test-work-dir/test-bpf-based-device-cgroup
> > 
> > somehow such mkdir succeeds and new directory appears:
> > /mnt/cgroup-test-work-dir/cgroup-test-work-dirtest-bpf-based-device-cgroup
> > 
> > Later cleanup via nftw("/mnt/cgroup-test-work-dir", ...);
> > doesn't walk this directory.
> > "rmdir /mnt/cgroup-test-work-dir" succeeds, but bpf program and
> > dangling cgroup stays in memory.
> > That's a separate issue on a cgroup side.
> 
> Purely cgroup side with regards to internal cleanup (which then as a
> side effect doesn't drop ref on the BPF prog)? Is a fix taken care of?

we're arguing whether it's a bug in the first place.
When cgroup_helpers.c were written my understanding was that
the sequence:
unshare(CLONE_NEWNS);
mount("none", CGROUP_MOUNT_PATH, "cgroup2", 0, NULL)
will give us fresh cgroup2 mount where the tests can run without affecting
other cgroup2s.
Turned out that cgroup2 hierarchy is still global here.
When newns is destroyed the cgroup2 fs gets unmounted, but internal
hierarchy stays around in arguably hidden form (though cgroup2 is no longer
mounted anywhere).

The next time we execute test_dev_cgroup it picks up the same tmp test
directory. New bpf program is loaded and attached to old cgroup dir.
That attach operation overrides old program and now new program
becomes hidden when newns is destroyed second time.

I thought the following diff:
-       if (unshare(CLONE_NEWNS)) {
+       if (unshare(CLONE_NEWNS | CLONE_NEWCGROUP)) {
should make cgroup behave like fresh hierarchy, but it didn't work either.