[libvirt] [PATCH v5 1/6] qemu-char: Add MSG_CMSG_CLOEXEC flag to recvmsg
Corey Bryant
coreyb at linux.vnet.ibm.com
Tue Jul 24 02:19:52 UTC 2012
On 07/23/2012 06:50 PM, Eric Blake wrote:
> 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).
>
Ugh, that's bad.
>> + }
>> +#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.
>
Thanks for the suggestion and catching this. I'll take this into
account in the next version.
--
Regards,
Corey
More information about the libvir-list
mailing list