Re: [PATCH 4/4] qemu_passt: Don't let passt fork off

On 2/15/23 08:50, Laine Stump wrote:

On 2/14/23 8:02 AM, Stefano Brivio wrote:

...

On Tue, 14 Feb 2023 12:51:22 +0100 Michal Privoznik wrote:

...

When passt starts it tries to do some security measures to restrict itself. For instance, it creates its own namespaces, umounts basically everything, drops capabilities, forks off to further restrict itself (the child is where all interesting work takes place now). This is sound, except it's causing two problems:

1) the PID file FD, which we leak into the passt process, gets     closed (and thus our virPidFile*() helpers see unlocked PID     file, which makes them think the process is gone),

I didn't realise this was the case, but giving passt write (unless I'm missing something) access to a file created by libvirtd doesn't look desirable to me.

...

...

2) the PID file no longer reflects true PID of the process.

Worse, the child calls setsid() so we can't even kill the whole process group. I mean, we can but it won't be any good.

I think that (incorrect PID in the pidfile) is  happening because Michal is using the original version of my patches that were pushed - I had mimicked the behavior of slirp, where libvirt deamonizes the new process. If that process then daemonizes itself, we have some sort of "double daemon"; libvirt has saved off the pid of what it thinks is going to be the final process, but then that process further forks and exits from the process whose pid libvirt saved. But because passt was cleaning up after itself I hadn't noticed the discrepancy in pids when testing.

Without going into all the details of the pidfile and locking and etc, I just want to say that if we can fork/exec dnsmasq and let it daemonize itself and create its own pidfile, then certainly we can do the same thing for passt. (and if there's a fundamental problem, then it's a fundamental problem for dnsmasq as well).

Alright. I think I have a solution that would please everybody involved. I'll post it tomorrow though. I need to test it thoroughly. We would be able to get passt's PID (which is needed not only for killing it, but also for CGroup placement), NOT use --foreground and still pass errors from it to users (that is unless logfile was specified, because unfortunately, --log-file and --stderr are mutually exclusive). Michal