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 <ler...@redhat.com> > Reviewed-by: Richard W.M. Jones <rjo...@redhat.com> > ---
Reviewed-by: Eric Blake <ebl...@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org _______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
