[RFC 0/1] Check for pid re-use before killing domain process

[manish.mishra]

On 11/10/22 10:17 pm, Daniel P. Berrangé wrote:
> On Tue, Oct 11, 2022 at 10:11:29PM +0530, manish.mishra wrote:
>> Thanks for review Jonathon, Daniel
>>
>> On 11/10/22 9:56 pm, Daniel P. Berrangé wrote:
>>> On Tue, Oct 11, 2022 at 11:20:00AM -0500, Jonathon Jongsma wrote:
>>>> I believe that pidfd syscalls were introduced in kernel 5.2. Judging by our
>>>> CI build setup, the oldest distrubution that we support is Alma Linux 8,
>>>> which still has kernel version 4.18.
>> I did not know that, will it okay to put it in some condional check. Pidfd
>> is used anyway to remove a very small window race, more important one
>> is start time comparison. As these kind of issues can be very easily
>> reproducible if domain process dies when libvirt is dead and by time
>> libvirt comes pid is re-used. So atleast start time checking reduce race
>> window even for older kernel.
> I presume you're seeing this on fairly old kernels ? On modern
> kernels, pid_max is ~4 billion, instead of 64k, so the chances
> of seeing PID reuse is tiny.
>
> If the risk is primarily from the situation where libvirtd was
> shutoff at the time QEMU stopped, then we ought to read
> /proc/$PID/stat in qemuProcessReconnect() and entirely skip
> any attempt to reconnect if we see the starttime has changed
> while we were stopped. Of course needs to be skipped on non-Linux.
>
> Also having an conditionally compiled pidfd check during the
> kill() path would be a second safety net, for modern Linux.
>
>>> We need to support FreeBSD / macOS for the QEMU driver too, which becomes
>>> the same problem.
>> Sorry Daniel i could not understand much of this, can you please give some
> Libvirt targets multiple platforms, not merely Linux. the QEMU driver
> is expected to run on Linux, FreeBSD and macOS, so cannot rely on
> Linux specific functionality - that has to be conditionally built.

Yes got it, this clarifies, thanks Daniel, I currently i had for non-windows

platform, sure i will make is for specific, also check for kernel version.

Are you okay to create a separate file for stime. I mean i have to keep

this keeping in mind that i can not change .pid file and libvirt can restart

any time so persisting in just domain xml by sharing stime with pipes

may not be full proof.

>
> With regards,
> Daniel