Re: [RFC][PATCH] net/bpfilter: Remove this broken and apparently unmantained

[Alexei Starovoitov <alexei.starovoitov@gmail.com>]

On Sun, Jun 14, 2020 at 09:51:09AM -0500, Eric W. Biederman wrote:
> 
> There are some asperations to use fork_usermode_blob but no commitments
> that I can see to actually use this code.

huh? I've listed three projects with concrete timelines that are going to use
fork_usermode_blob.

> If someone who cares can step up so other developers don't have to deal
> with the maintenance problems, then there is no problem in keeping the
> code.

That code has been there for two years and wasn't causing 'maintenance
problems'. Quite the opposite was happening. Initial way of embedding blob
into ko has changed quite a bit thanks to work from Masahiro.
See commit 8e75887d321d ("bpfilter: include bpfilter_umh in assembly instead of using objcopy")

What is happening that this bit of code is somehow in a way of some refactoring
that you're doing (I'm not even sure what kind of refactoring you have in
mind), but instead of working with the community on the best ways to do this
refactoring you're arguing for removal just to avoid tweaking few lines of code.

> Now there is one technical issue I see that has implications for how
> this gets fixed.  The current implementation requires that 2 copies
> of the user mode executable be kept.
> 
> int fork_usermode_blob(void *data, size_t len, struct umh_info *info);
> 
> 
> The function fork_usermode_blob is passed an array and a length.  Today
> that array is stored in .rodata.  Not in a init section where it could
> be discared.

It's a one line change in bpfilter_umh_blob.S to make it .init section,
but for bpfilter init may not work.
For some ko init is appropriate for some other it's not.

> Now userspace in general and exec in particular requires the executable
> to be in a mmapable file.  So fork_usermode_blob creates a mini
> filesystem that only supports one file and no file names and opens
> a file within it, and passes that open file to exec.
> 
> If creation of the filesystem and copying of the data can be separated
> from the actual execution of the code, then there will be no need to
> keep 2 copies of the executable in memory.  If the file was also given a
> name there would be no need for fork_usermode_blob to open the file.
> All fork_usermode_blob would need to do is make make it possible for
> exec to find that file.
> 
> The implification this has for fixing the issues with exec is that once
> the file has a name fork_usermode_blob no longer needs to preopen the
> file and call do_execve_file.  Instead fork_usermode_blob can call
> do_execve.  Which means do_execve_file and all of it's strange corner
> cases can go away.
> 
> We have all of the infrastructure to decode a cpio in init/initramfs.c
> so it would be practically no code at all to place the code into a
> filesystem instead of just into a file at startup time.  At which
> point it could be guaranteed that the section the filesystem lives in is
> an init section and is not used past the point of loading it into a
> filesystem.  Making the code use half the memory.

Could you please re-read the explanation just up the thread:
https://lore.kernel.org/bpf/20200613033821.l62q2ed5ligheyhu@ast-mbp/
that goes into detail how bpfilter is invoking this blob.
Now explain how initramfs could work?
How bpfilter can load its blob when bpfilter.ko was loaded into
the kernel a day after boot ? Where is initramfs?
bpfilter can be normal ko and builtin. In both cases it cannot rely on
a path. That path may not exist. initramfs is not present after boot.
Any path based approach has serious disadvantages.
The ko cannot rely on an external fs hieararchy. The ko is a self contained
object. It has kernel and user code. A blob inside ko is like another kernel
function of that particular ko, but it runs in user space. The root fs could
have been corrupted but ko needs to be operational if it was builtin.

Another reason why single fs (initramfs or other) doesn't work is multiple
ko-s. Theoretically all ko-s can agree on dir layout, but ko-s are built and
loaded at different times. Say we put all possibles blobs from all ko-s into
some new special fs that is available during the boot and after the boot. In
such case the majority of that ram is going to be wasted. Since ko-s may not
need that blob to run or ko-s may not even load, but ram is wasted anyway.
All these show stoppers with fs and path were considered two years ago
when design of user mode blobs was done.