On Thu, 2022-09-15 at 11:30 +0100, Lorenz Bauer wrote: > Hi list, > > Here is a summary of the talk I gave at LPC '22 titled "Closing the > BPF map permission loophole", with slides at [0]. > > Problem #1: Read-only fds can be modified via a BPF program > > 1. Craft a BPF program that executes bpf_map_update_elem(read-only > fd, ...) > 2. Load the program & execute it > > The reason is that the verifier only checks bpf_map->map_flags in > resolve_pseudo_ldimm64, but ignores fd->f_mode. > > Fixing this problem is complicated by the fact that a user may use > several distinct fds with differing permissions to refer to the same > map, but that the verifier internally only tracks unique struct > bpf_map. See [1]. Hi Lorenz thanks for the detailed report. Unfortunately, I could not attend your presentation at Linux Plumbers, and the recording is not yet available. So, I rely on this and your slides. I saw your fix #2, even if I didn't fully understand it. What do you think instead about converting the fd modes to map flags (e.g. BPF_F_RDONLY -> BPF_RDONLY_PROG), and we rely on the existing verifier behavior for the _PROG counterparts? In this way, it will be the verifier enforcing the decision made by security_bpf_map(). > Problem #2: Read-only fds can be "transmuted" into read-write fds via > map in map > > 1. BPF_MAP_UPDATE_ELEM(map in map fd, read-only fd) > 2. BPF_MAP_LOOKUP_ELEM(map in map fd) = read-write fd > > This was pointed out by Stanislav Fomichev during the LPC session. > I've not yet tried this myself. Didn't look into it yet. > Problem #3: Read-only fds can be transmuted into read-write fds via > object pinning > > 1. BPF_OBJ_PIN(read-only fd, /sys/fs/bpf/foo) > 2. BPF_OBJ_GET(/sys/fs/bpf/foo) = read-write fd > > The problem is with BPF_OBJ_PIN semantics: regardless of fd->f_mode, > pinning creates an inode that is owned by the current user, with mode > o=rw. Even if we made the inode o=r, a user / attacker can still use > chmod(2) to change it back to o=rw. Maybe I'm missing something, but I consider pinning more like adding a new reference to an eBPF object (like the ID). You are still subject to access control decision by security_bpf_map(), as for BPF_MAP_GET_FD_BY_ID(). Now, the security model is limited to two permissions (read, write). If we want to add a new one to decide whether or not a new reference can be added, we could revisit this. Roberto
