Jesper Dangaard Brouer <bro...@redhat.com> writes: > On Mon, 25 May 2020 14:15:32 +0200 > Toke Høiland-Jørgensen <t...@redhat.com> wrote: > >> David Ahern <dsah...@gmail.com> writes: >> >> > On 5/22/20 9:59 AM, Toke Høiland-Jørgensen wrote: >> >> David Ahern <dsah...@kernel.org> writes: >> >> >> >>> Implementation of Daniel's proposal for allowing DEVMAP entries to be >> >>> a device index, program id pair. Daniel suggested an fd to specify the >> >>> program, but that seems odd to me that you insert the value as an fd, but >> >>> read it back as an id since the fd can be closed. >> >> >> >> While I can be sympathetic to the argument that it seems odd, every >> >> other API uses FD for insert and returns ID, so why make it different >> >> here? Also, the choice has privilege implications, since the CAP_BPF >> >> series explicitly makes going from ID->FD a more privileged operation >> >> than just querying the ID. > > Sorry, I don't follow. > Can someone explain why is inserting an ID is a privilege problem?
See description here: https://lore.kernel.org/bpf/20200513230355.7858-1-alexei.starovoi...@gmail.com/ Specifically, this part: > Consolidating all CAP checks at load time makes security model similar to > open() syscall. Once the user got an FD it can do everything with it. > read/write/poll don't check permissions. The same way when bpf_prog_load > command returns an FD the user can do everything (including attaching, > detaching, and bpf_test_run). > > The important design decision is to allow ID->FD transition for > CAP_SYS_ADMIN only. What it means that user processes can run > with CAP_BPF and CAP_NET_ADMIN and they will not be able to affect each > other unless they pass FDs via scm_rights or via pinning in bpffs. >> > I do not like the model where the kernel changes the value the user >> > pushed down. >> >> Yet it's what we do in every other interface where a user needs to >> supply a program, including in prog array maps. So let's not create a >> new inconsistent interface here... > > I sympathize with Ahern on this. It seems very weird to insert/write > one value-type, but read another value-type. Yeah, I do kinda agree that it's a bit weird. But it's what we do everywhere else, so I think consistency wins out here. There might be an argument that maps should be different (because they're conceptually a read/write data structure not a syscall return value). But again, we already have a map type that takes prog IDs, and that already does the rewriting, so doing it different here would be even weirder... -Toke
