On Thu, Mar 23, 2023 at 01:10:11PM +0100, Laszlo Ersek wrote:
A prior patch highlighted the following leak: when any construction
step
fails in the parent after fork(), the child process is leaked.
We could plug the leak by inserting a new error handling section for
killing the child process and reaping it with waitpid(). However, it would
raise some new questions (what signal to send, ensure the child not ignore
that signal, hope that whatever process image we execute in the child
handle that signal properly, etc).
Instead, rearrange the steps in the CONNECT_COMMAND.START handler so that
fork() be the last operation in the parent process, on the construction
path. If fork() succeeds, let the entire handler succeed. For this, move
the fcntl() and nbd_internal_socket_create() calls above fork().
(The hoisting of the fcntl() calls is where we rely on the earlier
observation that O_NONBLOCK only applies to the parent-side end of the
socket pair, so it is safe to do before forking.)
The trouble in turn is that nbd_internal_socket_create() takes ownership
of the parent-side end of the socket pair (sv[0]). So once that function
returns successfully, we can't close sv[0] even on the error path -- we
close the "higher level" socket, and that invalidates sv[0] at once. Track
this ownership transfer with a new boolean flag therefore.
Signed-off-by: Laszlo Ersek <lersek(a)redhat.com>
Reviewed-by: Richard W.M. Jones <rjones(a)redhat.com>
---
Reviewed-by: Eric Blake <eblake(a)redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization:
qemu.org |
libvirt.org