Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> writes: > On Sat, Jun 13, 2020 at 8:33 AM Alexei Starovoitov > <alexei.starovoitov@xxxxxxxxx> wrote: >> >> On Sat, Jun 13, 2020 at 7:13 AM Eric W. Biederman <ebiederm@xxxxxxxxxxxx> wrote: >> > I am in the middle of cleaning up exec. Send the patches that address >> > the issues and make this mess not a maintenance issue, and I will be >> > happy to leave fork_usermode_blob alone. Otherwise I plan to just >> > remove the code for now as it is all dead at the moment. >> >> May be stop being a jerk first ? >> It's a Nack to remove the code. > > I'm happy to work on changes, but your removal threats must stop > before we can continue discussion. ok? I am looking for reasons to not remove the code. What I can't personally do is justify spending the time fixing unused code. The code is currently unused, and has an implementation that can be improved. All of which argues for removal on technical grounds. The implementation issues are a problem for maintaining other code so they need to be addressed, again that argues for removal on technical grounds. There are some asperations to use fork_usermode_blob but no commitments that I can see to actually use this code. 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. 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. 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. Eric
