[PATCH 00/14] Make the user mode driver code a better citizen

[Eric W. Biederman]

Alexei Starovoitov <alexei.starovoitov at gmail.com> writes:

> On Fri, Jun 26, 2020 at 07:51:41AM -0500, Eric W. Biederman wrote:
>> 
>> Asking for people to fix their bugs in this user mode driver code has
>> been remarkably unproductive.  So here are my bug fixes.
>> 
>> I have tested them by booting with the code compiled in and
>> by killing "bpfilter_umh" and running iptables -vnL to restart
>> the userspace driver.
>> 
>> I have split the changes into small enough pieces so they should be
>> easily readable and testable.  
>> 
>> The changes lean into the preexisting interfaces in the kernel and
>> remove special cases for user mode driver code in favor of solutions
>> that don't need special cases.  This results in smaller code with
>> fewer bugs.
>> 
>> At a practical level this removes the maintenance burden of the
>> user mode drivers from the user mode helper code and from exec as
>> the special cases are removed.
>> 
>> Similarly the LSM interaction bugs are fixed by not having unnecessary
>> special cases for user mode drivers.
>> 
>> Please let me know if you see any bugs.  Once the code review is
>> finished I plan to take this through my tree.
>
> I did a quick look and I like the cleanup. Thanks!

Good then we have a path forward.

> The end result looks good.
> The only problem that you keep breaking the build between patches,
> so series will not be bisectable.

Keep breaking?  There is an issue with patch 5/14 where the build breaks
when bpfilter is not enabled.  Do you see any others? I know I tested
each patch individually.  But I was only testing with CONFIG_BPFILTER
enabled so I missed one.

So there should not be things that break
but things slip through occassionally.

I will resend this shortly with the fix and any others that I can find.

> blob_to_mnt is a great idea. Much better than embedding fs you advocated earlier.

I was lazy and not overdesigning but I still suspect the blob will
benefit from becoming a cpio in the future.

> I'm swamped with other stuff today and will test the set Sunday/Monday
> with other patches that I'm working on.
> I'm not sure why you want to rename the interface. Seems
> pointless. But fine.

For maintainability I think the code very much benefits from a clear
separation between the user mode driver code from the user mode helper
code.

> As far as routing trees. Do you mind I'll take it via bpf-next ?
> As I said countless times we're working on bpf_iter using fork_blob.
> If you take this set via your tree we would need to wait the whole kernel release.
> Which is 8+ weeks before we can use the interface (due to renaming and overall changes).
> I'd really like to avoid this huge delay.
> Unless you can land it into 5.8-rc2 or rc3.

I also want to build upon this code.

How about when the review is done I post a frozen branch based on
v5.8-rc1 that you can merge into the bpf-next tree, and I can merge into
my branch.  That way we both can build upon this code.  That is the way
conflicts like this are usually handled.

Further I will leave any further enhancements to the user mode driver
infrastructure that people have suggested to you.

I will probably replace do_execve with a kernel_execve that doesn't need
set_fs() to copy the command line argument.  I haven't seen Christoph
Hellwig address that yet, and it looks pretty straight foward at this
point.


Eric