| Message ID | 20221107074222.1323017-1-houtao@huaweicloud.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Pin the start cgroup for cgroup iterator | expand |
On 11/6/22 11:42 PM, Hou Tao wrote: > From: Hou Tao <houtao1@huawei.com> > > Hi, > > The patchset tries to fix the potential use-after-free problem in cgroup > iterator. The problem is similar with the UAF problem fixed in map > iterator and the fixes is also similar: pinning the iterated resource > in .init_seq_private() and unpinning in .fini_seq_private(). Also adding > a test to demonstrate the problem. > > Not sure whether or not it will be helpful to add some comments for > .init_seq_private() to state that the implementation of > .init_seq_private() should not depend on iterator link to guarantee > the liveness of iterated object. Comments are always welcome. You added some comments in cgroup_iter init_seq_private(). Hopefully that can serve as an example so for future iterators we can search the code and remember to hold necessary references in init_seq_private() function.... > > Hou Tao (3): > bpf: Pin the start cgroup in cgroup_iter_seq_init() > selftests/bpf: Add cgroup helper remove_cgroup() > selftests/bpf: Add test for cgroup iterator on a dead cgroup > > kernel/bpf/cgroup_iter.c | 14 ++++ > tools/testing/selftests/bpf/cgroup_helpers.c | 19 +++++ > tools/testing/selftests/bpf/cgroup_helpers.h | 1 + > .../selftests/bpf/prog_tests/cgroup_iter.c | 78 +++++++++++++++++++ > 4 files changed, 112 insertions(+) >
Hi, On 11/8/2022 6:51 AM, Yonghong Song wrote: > > > On 11/6/22 11:42 PM, Hou Tao wrote: >> From: Hou Tao <houtao1@huawei.com> >> >> Hi, >> >> The patchset tries to fix the potential use-after-free problem in cgroup >> iterator. The problem is similar with the UAF problem fixed in map >> iterator and the fixes is also similar: pinning the iterated resource >> in .init_seq_private() and unpinning in .fini_seq_private(). Also adding >> a test to demonstrate the problem. >> >> Not sure whether or not it will be helpful to add some comments for >> .init_seq_private() to state that the implementation of >> .init_seq_private() should not depend on iterator link to guarantee >> the liveness of iterated object. Comments are always welcome. > > You added some comments in cgroup_iter init_seq_private(). Hopefully > that can serve as an example so for future iterators we can search > the code and remember to hold necessary references in init_seq_private() > function.... Another way to prevent such problem is to pin iterator link in iterator fd, but it introduce unnecessary dependency as said before, so hope the comments will be helpful. > >> >> Hou Tao (3): >> bpf: Pin the start cgroup in cgroup_iter_seq_init() >> selftests/bpf: Add cgroup helper remove_cgroup() >> selftests/bpf: Add test for cgroup iterator on a dead cgroup >> >> kernel/bpf/cgroup_iter.c | 14 ++++ >> tools/testing/selftests/bpf/cgroup_helpers.c | 19 +++++ >> tools/testing/selftests/bpf/cgroup_helpers.h | 1 + >> .../selftests/bpf/prog_tests/cgroup_iter.c | 78 +++++++++++++++++++ >> 4 files changed, 112 insertions(+) >> > > .
From: Hou Tao <houtao1@huawei.com> Hi, The patchset tries to fix the potential use-after-free problem in cgroup iterator. The problem is similar with the UAF problem fixed in map iterator and the fixes is also similar: pinning the iterated resource in .init_seq_private() and unpinning in .fini_seq_private(). Also adding a test to demonstrate the problem. Not sure whether or not it will be helpful to add some comments for .init_seq_private() to state that the implementation of .init_seq_private() should not depend on iterator link to guarantee the liveness of iterated object. Comments are always welcome. Hou Tao (3): bpf: Pin the start cgroup in cgroup_iter_seq_init() selftests/bpf: Add cgroup helper remove_cgroup() selftests/bpf: Add test for cgroup iterator on a dead cgroup kernel/bpf/cgroup_iter.c | 14 ++++ tools/testing/selftests/bpf/cgroup_helpers.c | 19 +++++ tools/testing/selftests/bpf/cgroup_helpers.h | 1 + .../selftests/bpf/prog_tests/cgroup_iter.c | 78 +++++++++++++++++++ 4 files changed, 112 insertions(+)