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@xxxxxxxxxx> >> >> 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(+) >> > > .