On Tue, Nov 15, 2022 at 5:37 PM Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> wrote: > > On Tue, Nov 15, 2022 at 11:16 AM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote: > > > > On 11/10/22 10:34 PM, Hou Tao wrote: > > > From: Hou Tao <houtao1@xxxxxxxxxx> > > > > > > For many bpf iterator (e.g., cgroup iterator), iterator link acquires > > > the reference of iteration target in .attach_target(), but iterator link > > > may be closed before or in the middle of iteration, so iterator will > > > need to acquire the reference of iteration target as well to prevent > > > potential use-after-free. To avoid doing the acquisition in > > > .init_seq_private() for each iterator type, just pin iterator link in > > > iterator. > > > > iiuc, a link currently will go away when all its fds closed and pinned file > > removed. After this change, the link will stay until the last iter is closed(). > > Before then, the user space can still "bpftool link show" and even get the > > link back by bpf_link_get_fd_by_id(). If this is the case, it would be useful > > to explain it in the commit message. > > > > and does this new behavior make sense when comparing with other link types? I think this is a unique problem in iter link. Because iter link is the only link type that can generate another object. > > One more question to the above... > > Does this change mean that pinned cgroup iterator in bpffs > would prevent cgroup removal? Yes, when attaching the program to cgroup, the cgroup iter link gets an extra ref of the cgroup. It puts that ref when detach. > So that cgroup cannot even become a dying cgroup ? > No. The cgroup will become offline and its corresponding kernfs node will be removed. The cgroup object is still accessible. > If so we can do that and an approach similar to init_seq_private > taken for map iterators is necessary here as well. > > Also pls target this kind of change to bpf-next especially > when there is a consideration to revert other fixes. > This kind of questionable fixes are not suitable for bpf tree > regardless of how long the "bug" was present.
