Re: [Libguestfs] [libnbd PATCH v3 11/19] CONNECT_COMMAND.START: sanitize close() calls in the child process
On Thu, Mar 23, 2023 at 01:10:08PM +0100, Laszlo Ersek wrote:
This code silently assumes that sv[1] falls outside of the the fd
set
{0,1} -- put differently, the code assumes that each dup2() call will
duplicate sv[1] to a file descriptor that is *different* from sv[1].
It is SOOO much easier to write code when you can assume a conforming
environment ;) (For comparison, look at GNU Coreutils which uses files
like "stdio--.h" that redefine functions like tmpfile() into
tmpfile_safer() which guarantee the resulting fd allocated by the end
of the function has been moved out of the way of the standard
descriptors, if the standard descriptors started life closed - it's a
lot of work, for very little gain if you have an environment that
won't even let you start a process that way).
Therefore:
- While valid, the assumption is not trivial. So, assert it in the child
process. Furthermore, because regular assert()'s in the parent process
may be easier to read for the user, assert a slightly more comprehensive
predicate about socketpair()'s output there, too.
- Remove the first two close() calls, which are superfluous.
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