Re: [PATCH v5 1/6] qemu-char: Add MSG_CMSG_CLOEXEC flag to recvmsg

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07/23/2012 07:08 AM, Corey Bryant wrote:
> Set the close-on-exec flag for the file descriptor received
> via SCM_RIGHTS.
> 

> +++ b/qemu-char.c
> @@ -2263,9 +2263,17 @@ static ssize_t tcp_chr_recv(CharDriverState *chr, char *buf, size_t len)
>      msg.msg_control = &msg_control;
>      msg.msg_controllen = sizeof(msg_control);
>  
> +#ifdef MSG_CMSG_CLOEXEC
> +    ret = recvmsg(s->fd, &msg, MSG_CMSG_CLOEXEC);
> +#else
>      ret = recvmsg(s->fd, &msg, 0);
> -    if (ret > 0 && s->is_unix)
> +    if (ret > 0) {
> +        qemu_set_cloexec(s->fd);

Wrong fd.  You aren't changing cloexec on the socket (s->fd), but on the
fd that was received via msg (which you don't know at this point in time).

> +    }
> +#endif
> +    if (ret > 0 && s->is_unix) {
>          unix_process_msgfd(chr, &msg);

Only here do you know what fd you received.

I would write it more like:

int flags = 0;
#ifdef MSG_CMSG_CLOEXEC
flags |= MSG_CMSG_CLOEXEC
#endif
ret = recvmsg(s->fd, &msg, flags);
if (ret > 0 && s->is_unix) {
    unix_process_msgfd(chr, &msg);
#ifndef MSG_CMSG_CLOEXEC
    qemu_set_cloexec(/* fd determined from msg */)
#endif
}

which almost implies that unix_process_msgfd() should be the function
that sets cloexec, but without wasting the time doing so if recvmsg
already did the job.

-- 
Eric Blake   eblake@xxxxxxxxxx    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]