[prev in list] [next in list] [prev in thread] [next in thread] 

List:       libguestfs
Subject:    Re: [Libguestfs] [libnbd PATCH v3 11/19] CONNECT_COMMAND.START: sanitize close() calls in the child 
From:       Eric Blake <eblake () redhat ! com>
Date:       2023-03-23 14:53:37
Message-ID: gbgazogkswds2qlhyhoqcrht2pdj62apwv7p5wcfwxrp36f5o7 () g4quc55o5ejs
[Download RAW message or body]

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@redhat.com>
> Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
> ---
>

Reviewed-by: Eric Blake <eblake@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

[prev in list] [next in list] [prev in thread] [next in thread]