Re: [PATCH bpf-next v2 06/15] bpf: Allow storing user kptr in map

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Mar 19, 2022 at 11:58:13PM IST, Alexei Starovoitov wrote:
> On Thu, Mar 17, 2022 at 05:29:48PM +0530, Kumar Kartikeya Dwivedi wrote:
> > Recently, verifier gained __user annotation support [0] where it
> > prevents BPF program from normally derefering user memory pointer in the
> > kernel, and instead requires use of bpf_probe_read_user. We can allow
> > the user to also store these pointers in BPF maps, with the logic that
> > whenever user loads it from the BPF map, it gets marked as MEM_USER. The
> > tag 'kptr_user' is used to tag such pointers.
> >
> >   [0]: https://lore.kernel.org/bpf/20220127154555.650886-1-yhs@xxxxxx
> >
> > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx>
> > ---
> >  include/linux/bpf.h   |  1 +
> >  kernel/bpf/btf.c      | 13 ++++++++++---
> >  kernel/bpf/verifier.c | 15 ++++++++++++---
> >  3 files changed, 23 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index 433f5cb161cf..989f47334215 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -163,6 +163,7 @@ enum {
> >  enum {
> >  	BPF_MAP_VALUE_OFF_F_REF    = (1U << 0),
> >  	BPF_MAP_VALUE_OFF_F_PERCPU = (1U << 1),
> > +	BPF_MAP_VALUE_OFF_F_USER   = (1U << 2),
> ...
> > +		} else if (!strcmp("kptr_user", __btf_name_by_offset(btf, t->name_off))) {
>
> I don't see a use case where __user pointer would need to be stored into a map.
> That pointer is valid in the user task context.
> When bpf prog has such pointer it can read user mem through it,
> but storing it for later makes little sense. The user context will certainly change.
> Reading it later from the map is more or less reading random number.
> Lets drop this patch until real use case arises.

In some cases the address may be fixed (e.g. user area registration similar to
rseq, that can be done between task and BPF program), as long as the task is
alive.

But the patch itself is trivial, so fine with dropping for now.

--
Kartikeya



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux