[Libguestfs] [PATCH libnbd] states: connect_command: Don't set O_NONBLOCK on socket passed to child.
Eric Blake
eblake at redhat.com
Sat May 25 20:30:18 UTC 2019
On 5/25/19 3:22 PM, Eric Blake wrote:
> On 5/25/19 1:33 PM, Richard W.M. Jones wrote:
>> I also made the code a bit more robust about closing the socket along
>> error paths.
>> ---
>> generator/states-connect.c | 21 ++++++++++++++++++---
>> 1 file changed, 18 insertions(+), 3 deletions(-)
>>
>
>> assert (!h->sock);
>> assert (h->argv);
>> assert (h->argv[0]);
>> - if (socketpair (AF_UNIX, SOCK_STREAM|SOCK_NONBLOCK|SOCK_CLOEXEC, 0,
>> - sv) == -1) {
>> + if (socketpair (AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0, sv) == -1) {
>
> Is it any easier to keep SOCK_NONBLOCK here and then clear O_NONBLOCK in
> the child process? It may matter if we try to port to a system that
> lacks SOCK_CLOEXEC (SOCK_NONBLOCK and SOCK_CLOEXEC were added at the
> same time). But we can deal with portability when someone reports a
> problem. For now the patch looks fine.
Hmm. Your use of SOCK_CLOEXEC here made me look for other fds that we
might inadvertently destroy or leak in a multi-threaded process that
does fork/exec (or even if the program linking against libnbd does
connect_command() in two separate threads on two different nbd objects).
I found:
lib/crypto.c: fp = fopen (pskfile, "r");
We need to use either fopen(pskfile, "re") (if libc is new-enough to
support "e" for O_CLOEXEC) or raw open(O_CLOEXEC) + fdopen() instead.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20190525/23b2d870/attachment.sig>
More information about the Libguestfs
mailing list