From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Mbd2m-00068e-3i for qemu-devel@nongnu.org; Thu, 13 Aug 2009 12:20:48 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Mbd2h-00065v-Kz for qemu-devel@nongnu.org; Thu, 13 Aug 2009 12:20:47 -0400 Received: from [199.232.76.173] (port=55965 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Mbd2h-00065Y-4h for qemu-devel@nongnu.org; Thu, 13 Aug 2009 12:20:43 -0400 Received: from fleet.cs.ualberta.ca ([129.128.22.22]:55988) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1Mbd2g-000466-Ng for qemu-devel@nongnu.org; Thu, 13 Aug 2009 12:20:43 -0400 Received: from fleet.cs.ualberta.ca (localhost.localdomain [127.0.0.1]) by fleet-spampd (Postfix) with ESMTP id 2FEF328014 for ; Thu, 13 Aug 2009 10:20:27 -0600 (MDT) Message-ID: <4A843D4A.5030306@cs.ualberta.ca> Date: Thu, 13 Aug 2009 10:20:26 -0600 From: Cam Macdonell MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 2/5] Add SCM_RIGHTS support to unix socket character devices References: <1248250235.2867.34.camel@blaa> <1248250302-28595-2-git-send-email-markmc@redhat.com> In-Reply-To: <1248250302-28595-2-git-send-email-markmc@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Mark McLoughlin Cc: qemu-devel@nongnu.org Mark McLoughlin wrote: > If a file descriptor is passed via a message with SCM_RIGHTS ancillary > data on a unix socket, store the file descriptor for use in the > chr_read() handler. Close the file descriptor if it was not used. > > The qemu_chr_get_msgfd() API provides access to the passed descriptor. > > Signed-off-by: Mark McLoughlin > --- > qemu-char.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- > qemu-char.h | 2 ++ > 2 files changed, 56 insertions(+), 1 deletions(-) > > diff --git a/qemu-char.c b/qemu-char.c > index 9886228..6e2cd31 100644 > --- a/qemu-char.c > +++ b/qemu-char.c > @@ -168,6 +168,11 @@ void qemu_chr_read(CharDriverState *s, uint8_t *buf, int len) > s->chr_read(s->handler_opaque, buf, len); > } > > +int qemu_chr_get_msgfd(CharDriverState *s) > +{ > + return s->get_msgfd ? s->get_msgfd(s) : -1; > +} > + > void qemu_chr_accept_input(CharDriverState *s) > { > if (s->chr_accept_input) > @@ -1832,6 +1837,7 @@ typedef struct { > int do_telnetopt; > int do_nodelay; > int is_unix; > + int msgfd; > } TCPCharDriver; > > static void tcp_chr_accept(void *opaque); > @@ -1907,20 +1913,61 @@ static void tcp_chr_process_IAC_bytes(CharDriverState *chr, > *size = j; > } > > +static int tcp_get_msgfd(CharDriverState *chr) > +{ > + TCPCharDriver *s = chr->opaque; > + > + return s->msgfd; > +} > + > #ifndef WIN32 > +static void unix_process_msgfd(CharDriverState *chr, struct msghdr *msg) > +{ > + TCPCharDriver *s = chr->opaque; > + struct cmsghdr *cmsg; > + > + for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) { > + int fd; > + > + if (cmsg->cmsg_len != CMSG_LEN(sizeof(int)) || > + cmsg->cmsg_level != SOL_SOCKET || > + cmsg->cmsg_type != SCM_RIGHTS) > + continue; > + > + fd = *((int *)CMSG_DATA(cmsg)); > + if (fd < 0) > + continue; > + > + if (s->msgfd != -1) > + close(s->msgfd); > + s->msgfd = fd; > + } > +} > + > static ssize_t tcp_chr_recv(CharDriverState *chr, char *buf, size_t len) > { > TCPCharDriver *s = chr->opaque; > struct msghdr msg = { 0, }; > struct iovec iov[1]; > + union { > + struct cmsghdr cmsg; > + char control[CMSG_SPACE(sizeof(int))]; > + } msg_control; > + ssize_t ret; > > iov[0].iov_base = buf; > iov[0].iov_len = len; > > msg.msg_iov = iov; > msg.msg_iovlen = 1; > + msg.msg_control = &msg_control; > + msg.msg_controllen = sizeof(msg_control); > + > + ret = recvmsg(s->fd, &msg, 0); > + if (ret > 0 && s->is_unix) > + unix_process_msgfd(chr, &msg); > > - return recvmsg(s->fd, &msg, 0); > + return ret; > } > #else > static ssize_t tcp_chr_recv(CharDriverState *chr, char *buf, size_t len) > @@ -1957,6 +2004,10 @@ static void tcp_chr_read(void *opaque) > tcp_chr_process_IAC_bytes(chr, s, buf, &size); > if (size > 0) > qemu_chr_read(chr, buf, size); > + if (s->msgfd != -1) { > + close(s->msgfd); > + s->msgfd = -1; > + } Hi Mark, I'm trying to understand the intended behaviour here so I can use your patch. In your comments for the patch it reads "store the file descriptor for use in the chr_read() handler. Close the file descriptor if it was not used." It seems that upon returning from the chr_read() handler the descriptor is closed if s->msgfd is not set to -1. Does this mean the handler should set the fd to -1 after retrieving it if the process wants to keep that fd open, otherwise it will be closed when chr_read() returns? Should tcp_get_msgfd() perhaps set s->msgfd to -1 after it is retrieved? Sort of a "read and clear" behaviour? Thanks, Cam > } > } > > @@ -2118,12 +2169,14 @@ static CharDriverState *qemu_chr_open_tcp(const char *host_str, > s->connected = 0; > s->fd = -1; > s->listen_fd = -1; > + s->msgfd = -1; > s->is_unix = is_unix; > s->do_nodelay = do_nodelay && !is_unix; > > chr->opaque = s; > chr->chr_write = tcp_chr_write; > chr->chr_close = tcp_chr_close; > + chr->get_msgfd = tcp_get_msgfd; > > if (is_listen) { > s->listen_fd = fd; > diff --git a/qemu-char.h b/qemu-char.h > index e1aa8db..77d4eda 100644 > --- a/qemu-char.h > +++ b/qemu-char.h > @@ -51,6 +51,7 @@ struct CharDriverState { > int (*chr_write)(struct CharDriverState *s, const uint8_t *buf, int len); > void (*chr_update_read_handler)(struct CharDriverState *s); > int (*chr_ioctl)(struct CharDriverState *s, int cmd, void *arg); > + int (*get_msgfd)(struct CharDriverState *s); > IOEventHandler *chr_event; > IOCanRWHandler *chr_can_read; > IOReadHandler *chr_read; > @@ -81,6 +82,7 @@ void qemu_chr_reset(CharDriverState *s); > void qemu_chr_initial_reset(void); > int qemu_chr_can_read(CharDriverState *s); > void qemu_chr_read(CharDriverState *s, uint8_t *buf, int len); > +int qemu_chr_get_msgfd(CharDriverState *s); > void qemu_chr_accept_input(CharDriverState *s); > void qemu_chr_info(Monitor *mon); >