Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> writes: > On Tue, Apr 14, 2020 at 3:32 AM Toke Høiland-Jørgensen <toke@xxxxxxxxxx> wrote: >> >> Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> writes: >> >> >> > After that, one can pin bpf_link temporarily and re-open it as >> >> > writable one, provided CAP_DAC_OVERRIDE capability is present. All >> >> > that works already, because pinned bpf_link is just a file, so one can >> >> > do fchmod on it and all that will go through normal file access >> >> > permission check code path. >> >> >> >> Ah, I did not know that was possible - I was assuming that bpffs was >> >> doing something special to prevent that. But if not, great! >> >> >> >> > Unfortunately, just re-opening same FD as writable (which would >> >> > be possible if fcntl(fd, F_SETFL, S_IRUSR >> >> > S_IWUSR) was supported on Linux) without pinning is not possible. >> >> > Opening link from /proc/<pid>/fd/<link-fd> doesn't seem to work >> >> > either, because backing inode is not BPF FS inode. I'm not sure, but >> >> > maybe we can support the latter eventually. But either way, I think >> >> > given this is to be used for manual troubleshooting, going through few >> >> > extra hoops to force-detach bpf_link is actually a good thing. >> >> >> >> Hmm, I disagree that deliberately making users jump through hoops is a >> >> good thing. Smells an awful lot like security through obscurity to me; >> >> and we all know how well that works anyway... >> > >> > Depends on who users are? bpftool can implement this as one of >> > `bpftool link` sub-commands and allow human operators to force-detach >> > bpf_link, if necessary. >> >> Yeah, I would expect this to be the common way this would be used: built >> into tools. >> >> > I think applications shouldn't do this (programmatically) at all, >> > which is why I think it's actually good that it's harder and not >> > obvious, this will make developer think again before implementing >> > this, hopefully. For me it's about discouraging bad practice. >> >> I guess I just don't share your optimism that making people jump through >> hoops will actually discourage them :) > > I understand. I just don't see why would anyone have to implement this > at all and especially would think it's a good idea to begin with? > >> >> If people know what they are doing it should be enough to document it as >> discouraged. And if they don't, they are perfectly capable of finding >> and copy-pasting the sequence of hoop-jumps required to achieve what >> they want, probably with more bugs added along the way. >> >> So in the end I think that all you're really achieving is annoying >> people who do have a legitimate reason to override the behaviour (which >> includes yourself as a bpftool developer :)). That's what I meant by the >> 'security through obscurity' comment. > > Can I please get a list of real examples of legitimate reasons to > override this behavior? Primarily, I expect that this would be built into admin tools (like bpftool as you suggested). I just don't see why such tools should be made to do the whole pin/reopen dance (which, BTW, adds an implicit dependency on having a mounted bpffs) when we could just add a capability check directly in bpf_link_get_fd_by_id()? -Toke
