On Fri, 2022-11-18 at 09:44 +0100, Roberto Sassu wrote:
> On 11/16/2022 6:12 PM, Casey Schaufler wrote:
> > On 11/16/2022 7:47 AM, Roberto Sassu wrote:
> > > From: Roberto Sassu <roberto.sassu@xxxxxxxxxx>
> > >
> > > eBPF allows certain types of eBPF programs to modify the return value of
> > > the functions they attach to. This is used for example by BPF LSM to let
> > > security modules make their decision on LSM hooks.
> > >
> > > The JITed code looks like the following:
> > >
> > > ret = bpf_lsm_inode_permission_impl1(); // from a security module
> > > if (ret)
> > > goto out;
> > >
> > > ..
> > >
> > > ret = bpf_lsm_inode_permission_implN(); // from a security module
> > > if (ret)
> > > goto out;
> > >
> > > ret = bpf_lsm_inode_permission(); // in the kernel, returns DEFAULT
> > > out:
> > >
> > > If ret is not zero, the attachment points of BPF LSM are not executed. For
> > > this reason, the return value check cannot be done there.
> > >
> > > Instead, the idea is to use the LSM_HOOK() macro to define a per-hook check
> > > function.
> > >
> > > Whenever an eBPF program attaches to an LSM hook, the eBPF verifier
> > > resolves the address of the check function (whose name is
> > > bpf_lsm_<hook name>_ret()) and adds a call to that function just after the
> > > out label. If the return value is illegal, the check function changes it
> > > back to the default value defined by the LSM infrastructure:
> > >
> > > ..
> > >
> > > out:
> > > ret = bpf_lsm_inode_permission_ret(ret);
> >
> > As I've mentioned elsewhere, the return value is a small part of
> > the problem you have with eBPF programs and the BPF LSM. Because
> > the LSM infrastructure is inconsistent with regard to return codes,
> > values returned in pointers and use of secids there is no uniform
> > mechanism that I can see to address the "legitimate return" problem.
> >
> > Lets look at one of the ickyest interfaces we have, security_getprocattr().
> > It returns the size of a string that it has allocated. It puts the
> > pointer to the allocated buffer into a char **value that was passed to it.
> > If bpf_getprocattr() returns a positive number and sets value to NULL Bad
> > Things can happen. If the return value is greater than the size allocated
> > ditto. If it returns an error but allocates a string you get a memory leak.
>
> I hope I understood how it works correctly, but you cannot modify
> directly data accessible from a pointer provided as parameter by the LSM
> hook you attach to. The pointer is treated as scalar value and the eBPF
> verifier detects any attempt to dereference as an illegal access. The
> only way to modify such data is through helpers that need to be properly
> declared to be usable by eBPF programs.
I wanted to double check about accessing the LSM hook arguments from an
eBPF program. I checked what it could prevent to access them.
First, in kernel/bpf/btf.c:
if (!btf_type_is_struct(t)) {
bpf_log(log,
"func '%s' arg%d type %s is not a struct\n",
If the argument is not a struct, it is not accessible.
Second, if a btf_struct_access method has not been defined for a
structure, only read can be done (kernel/bpf/verifier.c):
if (env->ops->btf_struct_access) {
ret = env->ops->btf_struct_access(...);
} else {
if (atype != BPF_READ) {
verbose(env, "only read is supported\n");
return -EACCES;
}
I found four:
net/bpf/bpf_dummy_struct_ops.c: .btf_struct_access =
bpf_dummy_ops_btf_struct_access,
net/core/filter.c: .btf_struct_access =
tc_cls_act_btf_struct_access,
net/core/filter.c: .btf_struct_access =
xdp_btf_struct_access,
net/ipv4/bpf_tcp_ca.c: .btf_struct_access =
bpf_tcp_ca_btf_struct_access,
Anything else?
Thanks
Roberto
