linux-kernel - Re: [PATCH RFC bpf-next 0/2] Mmapable task local storage.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 29 Mar 2022 14:45:36 -0700
From:   Martin KaFai Lau <kafai@...com>
To:     Hao Luo <haoluo@...gle.com>
Cc:     Kumar Kartikeya Dwivedi <memxor@...il.com>,
        Yonghong Song <yhs@...com>,
        Alexei Starovoitov <ast@...nel.org>,
        Andrii Nakryiko <andrii@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        KP Singh <kpsingh@...nel.org>,
        Song Liu <songliubraving@...com>, bpf@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC bpf-next 0/2] Mmapable task local storage.

On Tue, Mar 29, 2022 at 10:43:42AM -0700, Hao Luo wrote:
> On Tue, Mar 29, 2022 at 2:37 AM Kumar Kartikeya Dwivedi
> <memxor@...il.com> wrote:
> >
> > On Mon, Mar 28, 2022 at 11:16:15PM IST, Hao Luo wrote:
> > > On Mon, Mar 28, 2022 at 10:39 AM Hao Luo <haoluo@...gle.com> wrote:
> > > >
> > > > Hi Yonghong,
> > > >
> > > > On Fri, Mar 25, 2022 at 12:16 PM Yonghong Song <yhs@...com> wrote:
> > > > >
> > > > > On 3/24/22 4:41 PM, Hao Luo wrote:
> > > > > > Some map types support mmap operation, which allows userspace to
> > > > > > communicate with BPF programs directly. Currently only arraymap
> > > > > > and ringbuf have mmap implemented.
> > > > > >
> > > > > > However, in some use cases, when multiple program instances can
> > > > > > run concurrently, global mmapable memory can cause race. In that
> > > > > > case, userspace needs to provide necessary synchronizations to
> > > > > > coordinate the usage of mapped global data. This can be a source
> > > > > > of bottleneck.
> > > > >
> > > > > I can see your use case here. Each calling process can get the
> > > > > corresponding bpf program task local storage data through
> > > > > mmap interface. As you mentioned, there is a tradeoff
> > > > > between more memory vs. non-global synchronization.
> > > > >
> > > > > I am thinking that another bpf_iter approach can retrieve
> > > > > the similar result. We could implement a bpf_iter
> > > > > for task local storage map, optionally it can provide
> > > > > a tid to retrieve the data for that particular tid.
> > > > > This way, user space needs an explicit syscall, but
> > > > > does not need to allocate more memory than necessary.
> > > > >
> > > > > WDYT?
> > > > >
> > > >
> > > > Thanks for the suggestion. I have two thoughts about bpf_iter + tid and mmap:
> > > >
> > > > - mmap prevents the calling task from reading other task's value.
> > > > Using bpf_iter, one can pass other task's tid to get their values. I
> > > > assume there are two potential ways of passing tid to bpf_iter: one is
> > > > to use global data in bpf prog, the other is adding tid parameterized
> > > > iter_link. For the first, it's not easy for unpriv tasks to use. For
> > > > the second, we need to create one iter_link object for each interested
> > > > tid. It may not be easy to use either.
> > > >
> > > > - Regarding adding an explicit syscall. I thought about adding
> > > > write/read syscalls for task local storage maps, just like reading
> > > > values from iter_link. Writing or reading task local storage map
> > > > updates/reads the current task's value. I think this could achieve the
> > > > same effect as mmap.
> > > >
> > >
> > > Actually, my use case of using mmap on task local storage is to allow
> > > userspace to pass FDs into bpf prog. Some of the helpers I want to add
> > > need to take an FD as parameter and the bpf progs can run
> > > concurrently, thus using global data is racy. Mmapable task local
> > > storage is the best solution I can find for this purpose.
Some more details is needed about the use case.  As long as there is
storage local to an individual task, racing within this one task's
specific storage is a non issue?

The patch 2 example is doable with the current api and is pretty far from
the above use case description.  The existing bpf_map_update_elem() and
bpf_map_lookup_elem() can not solve your use case?

or the current bpf_map_{update,lookup}_elem() works but
prefer a direct data read/write interface?

btw, how delete is going to look like ?

and do you see the mmap could be used with sk and inode storage instead
of the 'current' task?

> > >
> > > Song also mentioned to me offline, that mmapable task local storage
> > > may be useful for his use case.
> > >
> > > I am actually open to other proposals.
If the arraymap is local to an individual task, does it solve
your use case?  Have you thought about storing an arraymap (which is mmap-able)
in the local storage?  That could then be used to store ringbuf map and
other bpf maps in local storage.  It is logically similar to map-in-map.
The outer map is the pseudo local storage map here.

> > >
> >
> > You could also use a syscall prog, and use bpf_prog_test_run to update local
> > storage for current. Data can be passed for that specific prog invocation using
> > ctx. You might have to enable bpf_task_storage helpers in it though, since they
> > are not allowed to be called right now.
> >
> 
> The loading process needs CAP_BPF to load bpf_prog_test_run. I'm
> thinking of allowing any thread including unpriv ones to be able to
> pass data to the prog and update their own storage.