On Tue, Mar 22, 2022 at 11:28:26AM IST, Andrii Nakryiko wrote:
> On Sun, Mar 20, 2022 at 8:55 AM Kumar Kartikeya Dwivedi
> <memxor@xxxxxxxxx> wrote:
> >
> > While we can guarantee that even for unreferenced kptr, the object
> > pointer points to being freed etc. can be handled by the verifier's
> > exception handling (normal load patching to PROBE_MEM loads), we still
> > cannot allow the user to pass these pointers to BPF helpers and kfunc,
> > because the same exception handling won't be done for accesses inside
> > the kernel. The same is true if a referenced pointer is loaded using
> > normal load instruction. Since the reference is not guaranteed to be
> > held while the pointer is used, it must be marked as untrusted.
> >
> > Hence introduce a new type flag, PTR_UNTRUSTED, which is used to mark
> > all registers loading unreferenced and referenced kptr from BPF maps,
> > and ensure they can never escape the BPF program and into the kernel by
> > way of calling stable/unstable helpers.
> >
> > In check_ptr_to_btf_access, the !type_may_be_null check to reject type
> > flags is still correct, as apart from PTR_MAYBE_NULL, only MEM_USER,
> > MEM_PERCPU, and PTR_UNTRUSTED may be set for PTR_TO_BTF_ID. The first
> > two are checked inside the function and rejected using a proper error
> > message, but we still want to allow dereference of untrusted case.
> >
> > Also, we make sure to inherit PTR_UNTRUSTED when chain of pointers are
> > walked, so that this flag is never dropped once it has been set on a
> > PTR_TO_BTF_ID (i.e. trusted to untrusted transition can only be in one
> > direction).
> >
> > In convert_ctx_accesses, extend the switch case to consider untrusted
> > PTR_TO_BTF_ID in addition to normal PTR_TO_BTF_ID for PROBE_MEM
> > conversion for BPF_LDX.
> >
> > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx>
> > ---
> > include/linux/bpf.h | 10 +++++++++-
> > kernel/bpf/verifier.c | 34 +++++++++++++++++++++++++++-------
> > 2 files changed, 36 insertions(+), 8 deletions(-)
> >
>
> [...]
>
> > - if (reg->type != PTR_TO_BTF_ID && reg->type != PTR_TO_BTF_ID_OR_NULL)
> > - goto bad_type;
> > + if (off_desc->flags & BPF_MAP_VALUE_OFF_F_REF) {
> > + if (reg->type != PTR_TO_BTF_ID &&
> > + reg->type != (PTR_TO_BTF_ID | PTR_MAYBE_NULL))
> > + goto bad_type;
> > + } else { /* only unreferenced case accepts untrusted pointers */
> > + if (reg->type != PTR_TO_BTF_ID &&
> > + reg->type != (PTR_TO_BTF_ID | PTR_MAYBE_NULL) &&
> > + reg->type != (PTR_TO_BTF_ID | PTR_UNTRUSTED) &&
> > + reg->type != (PTR_TO_BTF_ID | PTR_MAYBE_NULL | PTR_UNTRUSTED))
>
> use base_type(), Luke! ;)
>
Ack, will switch.
> > + goto bad_type;
> > + }
> >
> > if (!btf_is_kernel(reg->btf)) {
> > verbose(env, "R%d must point to kernel BTF\n", regno);
>
> [...]
--
Kartikeya
