On Mon, Apr 14, 2025 at 4:46 PM Blaise Boscaccy
<bbosca...@linux.microsoft.com> wrote:
> Paul Moore <p...@paul-moore.com> writes:
> > On Apr 4, 2025 Blaise Boscaccy <bbosca...@linux.microsoft.com> wrote:
...
> >> +static int hornet_check_binary(struct bpf_prog *prog, union bpf_attr
> >> *attr,
> >> + struct hornet_maps *maps)
> >> +{
> >> + struct file *file = get_task_exe_file(current);
> >> + const unsigned long markerlen = sizeof(EBPF_SIG_STRING) - 1;
> >> + void *buf = NULL;
> >> + size_t sz = 0, sig_len, prog_len, buf_sz;
> >> + int err = 0;
> >> + struct module_signature sig;
> >> +>> + buf_sz = kernel_read_file(file, 0, &buf, INT_MAX, &sz,
> >> READING_EBPF);
> >> + fput(file);
> >> + if (!buf_sz)
> >> + return -1;
> >
> > I'm pretty sure I asked you about this already off-list, but I can't
> > remember the answer so I'm going to bring this up again :)
> >
> > This file read makes me a bit nervous about a mismatch between the
> > program copy operation done in the main BPF code and the copy we do
> > here in kernel_read_file(). Is there not some way to build up the
> > buffer with the BPF program from the attr passed into this function
> > (e.g. attr.insns?)?
> >
>
> There is. That would require modifying the BPF_PROG_LOAD subcommand
> along with modifying the skeletobn generator to use it. I don't know if
> there is enough buy-in from the ebpf developers to do that
> currently. Tacking the signature to the end of of the light-skeleton
> binary allows Hornet to operate without modifying the bpf subsystem and
> is very similar to how module signing currently works. Modules have the
> advantage of having a working in-kernel loader, which makes all of this
> a non-issue with modules.
>
> > If there is some clever reason why all of this isn't an issue, it might
> > be a good idea to put a small comment above the kernel_read_file()
> > explaining why it is both safe and good.
> >
>
> Will do. I don't see this being an issue ...
My mistake, I spaced out a bit when looking at this and for some
reason thought it was reading the BPF program/lskel itself and not the
signature, meaning that the BPF that was being checked by Hornet was
not necessarily the same BPF that was actually loaded. However,
that's not the case here, as you rightly point out it is just the
signature that is being read by the kernel_read_file() which shouldn't
be an issue.
Thanks for the correction and sorry for the noise :)
--
paul-moore.com
