Re: [PATCH 1/1] security: Propagate universal pointer data in bpf hooks

Paul Moore [off-list ref] writes:

On Wed, Feb 26, 2025 at 2:06 AM Song Liu [off-list ref] wrote:

quoted

On Tue, Feb 25, 2025 at 4:31 PM Blaise Boscaccy
[off-list ref] wrote:

quoted

Certain bpf syscall subcommands are available for usage from both
userspace and the kernel. LSM modules or eBPF gatekeeper programs may
need to take a different course of action depending on whether or not
a BPF syscall originated from the kernel or userspace.

Additionally, some of the bpf_attr struct fields contain pointers to
arbitrary memory. Currently the functionality to determine whether or
not a pointer refers to kernel memory or userspace memory is exposed
to the bpf verifier, but that information is missing from various LSM
hooks.

Here we augment the LSM hooks to provide this data, by simply passing
the corresponding universal pointer in any hook that contains already
contains a bpf_attr struct that corresponds to a subcommand that may
be called from the kernel.

I think this information is useful for LSM hooks.

I've only looked at it quickly, but so far it seems reasonable.  I'm
going to take a closer look today.

quoted

Question: Do we need a full bpfptr_t for these hooks, or just a boolean
"is_kernel or not"?

I may be misunderstanding the patch, but what if we swapped the
existing 'union bpf_attr' parameter for a 'bpfptr_t' parameter?  That
would allow for both kernel and usermode pointers, complete with a
'is_kernel' flag; or am I missing something (likely)?

-- 
paul-moore.com

bpfptr_t is just a typedef for a sockptr_t, which contains a void
pointer and bool, so if we replaced bpf_attr with it, we might lose a
bit of type safety going that route.

In syscall.c a most of the subcommand handlers have a

static int bpf_foo(union bpf_attr *attr, bpfptr_t uattr);

pattern that is used. I was trying to mimic for this patch.

The actual parts where the is_kernel flag gets used currently, is for
pointer chasing/copy stuff, e.g.

make_bpfptr(attr->insns, uattr.is_kernel)
make_bpfptr(attr->license, uattr.is_kernel)
make_bpfptr(attr->fd_array, uattr.is_kernel)

and subcommand structs may contain multiple pointers.

-blaise