[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f1c18c3b37b36d7550a467f4fb03a0f15e7647d4.camel@huaweicloud.com>
Date: Mon, 21 Nov 2022 16:31:29 +0100
From: Roberto Sassu <roberto.sassu@...weicloud.com>
To: Casey Schaufler <casey@...aufler-ca.com>, ast@...nel.org,
daniel@...earbox.net, andrii@...nel.org, martin.lau@...ux.dev,
song@...nel.org, yhs@...com, john.fastabend@...il.com,
kpsingh@...nel.org, sdf@...gle.com, haoluo@...gle.com,
jolsa@...nel.org, revest@...omium.org, jackmanb@...omium.org,
paul@...l-moore.com, jmorris@...ei.org, serge@...lyn.com
Cc: bpf@...r.kernel.org, linux-security-module@...r.kernel.org,
linux-kernel@...r.kernel.org,
Roberto Sassu <roberto.sassu@...wei.com>
Subject: Re: [PoC][PATCH] bpf: Call return value check function in the JITed
code
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@...wei.com>
> > >
> > > 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
Powered by blists - more mailing lists