| Message ID | 1343048885-1701-2-git-send-email-coreyb@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
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.
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.
diff --git a/qemu-char.c b/qemu-char.c index c2aaaee..eedf66d 100644 --- a/qemu-char.c +++ 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); + } +#endif + if (ret > 0 && s->is_unix) { unix_process_msgfd(chr, &msg); + } return ret; }
Set the close-on-exec flag for the file descriptor received via SCM_RIGHTS. v4 -This patch is new in v4 (eblake@redhat.com) v5 -Fallback to FD_CLOEXEC if MSG_CMSG_CLOEXEC is not available (eblake@redhat.com, stefanha@linux.vnet.ibm.com) Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com> --- qemu-char.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)