On Thu, 2025-04-24 at 16:41 -0700, Alexei Starovoitov wrote: > On Wed, Apr 23, 2025 at 7:12 AM James Bottomley > <james.bottom...@hansenpartnership.com> wrote: > > On Mon, 2025-04-21 at 13:12 -0700, Alexei Starovoitov wrote: > > [...] > > > Calling bpf_map_get() and > > > map->ops->map_lookup_elem() from a module is not ok either. > > > > I don't understand this objection. > > Consider an LSM that hooks into security_bprm_*(bprm), > parses something in linux_binprm, then > struct file *file = > fd_file(fdget(some_random_file_descriptor_in_current)); > file->f_op->read(..); > > Would VFS maintainers approve such usage ?
This is a bit off topic from the request for clarification but: It's somewhat standard operating procedure for LSMs. Some do make decisions entirely within the data provided by the hook, but some need to take external readings, like selinux or IMA consulting the policy in the xattr or apparmor the one in the tree etc. Incidentally, none of them directly does a file->f_op->read(); they all use the kernel_read_file() API which is exported from the vfs for that purpose. > More so, your LSM does > file = get_task_exe_file(current); > kernel_read_file(file, ...); > > This is even worse. > You've corrupted the ELF binary with extra garbage at the end. > objdump/elfutils will choke on it and you're lucky that binfmt_elf > still loads it. > The whole approach is a non-starter. It's the same approach we use to create kernel modules: ELF with an appended signature. If you recall the kernel summit discussions about it, the reason that was chosen for modules is because it's easy and the ELF processor simply ignores any data in the file that's not described by the header (which means the ELF tools you refer to above are fine with this if you actually try them). But it you really want the signature to be part of the ELF, then the patch set can do what David Howells first suggested for modules: it can simply put the appended signature into an unloaded ELF section. > > The program just got passed in to bpf_prog_load() as a set of > > attributes which, for a light skeleton, directly contain the code > > as a blob and have the various BTF relocations as a blob in a > > single element array map. I think everyone agrees that the > > integrity of the program would be compromised by modifications to > > the relocations, so the security_bpf_prog_load() hook can't make an > > integrity determination without examining both. If the hook can't > > use the bpf_maps.. APIs directly is there some other API it should > > be using to get the relocations, or are you saying that the > > security_bpf_prog_load() hook isn't fit for purpose and it should > > be called after the bpf core has loaded the relocations so they can > > be provided to the hook as an argument? > > No. As I said twice already the only place to verify program > signature is a bpf subsystem itself. The above argument is actually independent of signing. However, although we have plenty of subsystems that verify their own signatures, it's perfectly valid for a LSM to do it as well: IMA is one of the oldest LSMs and it's been verifying signatures over binaries and text files since it was first created. > Hacking into bpf internals from LSM, BPF-LSM program, > or any other kernel subsystem is a no go. All LSMs depend to some extent on the internals of the subsystem where the hook is ... the very structures passed into them are often internal to that subsystem. The problem you didn't address was that some of the information necessary to determine the integrity properties in the bpf hook is in a map file descriptor. Since the map merely wraps a single blob of data, that could easily be passed in to the hook instead of having the LSM extract it from the map. How the hook gets the data is an internal implementation detail of the kernel that can be updated later. > > The above, by the way, is independent of signing, because it > > applies to any determination that might be made in the > > security_bpf_prog_load() hook regardless of purpose. > > security_bpf_prog_load() should not access bpf internals. > That LSM hook sees the following: > security_bpf_prog_load(struct bpf_prog *prog, union bpf_attr *attr, > struct bpf_token *token, bool kernel); > > LSM can look into uapi things there. Is that the misunderstanding? That's not how LSMs work: they are not bound by only the UAPI, they are in kernel and have full access to the kernel API so they can introspect stuff and make proper determinations. > Like prog->sleepable, prog->tag, prog->aux->name, > but things like prog->aux->jit_data or prog->aux->used_maps > are not ok to access. > If in doubt, ask on the mailing list. I am aren't I? At least the bpf is one of the lists cc'd on this. Regards, James
