Re: [Qemu-devel] [PATCH 1/6] qemu-fd-exchange: provide common methods for exchange fd

[Lei Li]

On 01/17/2014 06:02 PM, Daniel P. Berrange wrote:
> On Wed, Jan 08, 2014 at 05:12:51PM +0800, Lei Li wrote:
> > Signed-off-by: Lei Li <address@hidden>
> > ---
> >   include/qemu/fd-exchange.h |   25 +++++++++++
> >   util/Makefile.objs         |    1 +
> >   util/qemu-fd-exchange.c    |   97 ++++++++++++++++++++++++++++++++++++++++++++
> >   3 files changed, 123 insertions(+), 0 deletions(-)
> >   create mode 100644 include/qemu/fd-exchange.h
> >   create mode 100644 util/qemu-fd-exchange.c
> >
> > diff --git a/include/qemu/fd-exchange.h b/include/qemu/fd-exchange.h
> > new file mode 100644
> > index 0000000..6929026
> > --- /dev/null
> > +++ b/include/qemu/fd-exchange.h
> > @@ -0,0 +1,25 @@
> > +/*
> > + * Internel common methods for exchange of FD
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2.  See
> > + * the COPYING file in the top-level directory.
> > + *
> > + */
> > +
> > +#ifndef FD_EXCHANGE_H
> > +#define FD_EXCHANGE_H
> > +
> > +#include <sys/socket.h>
> > +
> > +union MsgControl {
> > +    struct cmsghdr cmsg;
> > +    char control[CMSG_SPACE(sizeof(int))];
> > +};
> > +
> > +ssize_t qemu_send_with_fd(int sockfd, int passed_fd,
> > +                          const void *buf, size_t len);
> > +
> > +ssize_t qemu_recv_with_fd(int sockfd, int *passed_fd,
> > +                          void *buf, size_t len);
> > +
> > +#endif
> > diff --git a/util/Makefile.objs b/util/Makefile.objs
> > index af3e5cb..2fb42bf 100644
> > --- a/util/Makefile.objs
> > +++ b/util/Makefile.objs
> > @@ -13,3 +13,4 @@ util-obj-y += hexdump.o
> >   util-obj-y += crc32c.o
> >   util-obj-y += throttle.o
> >   util-obj-y += getauxval.o
> > +util-obj-y += qemu-fd-exchange.o
> > diff --git a/util/qemu-fd-exchange.c b/util/qemu-fd-exchange.c
> > new file mode 100644
> > index 0000000..70a3206
> > --- /dev/null
> > +++ b/util/qemu-fd-exchange.c
> > @@ -0,0 +1,97 @@
> > +/*
> > + * Internel common methods for exchange of FD
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2.  See
> > + * the COPYING file in the top-level directory.
> > + *
> > + */
> > +
> > +#include "qemu/fd-exchange.h"
> > +#include "qemu-common.h"
> > +
> > +
> > +ssize_t qemu_send_with_fd(int sockfd, int passed_fd,
> > +                          const void *buf, size_t len)
> > +{
> > +    struct msghdr msg;
> > +    struct iovec iov;
> > +    struct cmsghdr *cmsg;
> > +    union MsgControl msg_control;
> > +    int retval;
> > +
> > +    iov.iov_base = (int *)buf;
> > +    iov.iov_len = len;
> > +
> > +    memset(&msg, 0, sizeof(msg));
> > +    msg.msg_iov = &iov;
> > +    msg.msg_iovlen = len;
> > +    msg.msg_control = &msg_control;
> > +    msg.msg_controllen = sizeof(msg_control);
> > +
> > +    if (passed_fd < 0) {
> > +        *(int *)buf = passed_fd;
> You are casting 'char *buf' to an 'int *' but many of the
> callers only pass in a pointer to a 'char buf[1]'. So you
> are overflowing the array and also likely causing alignment
> violations on ARM platforms.

You are right, will fix it.

Thanks.

> > +    } else {
> > +        msg.msg_control = &msg_control;
> > +        msg.msg_controllen = sizeof(msg_control);
> > +
> > +        cmsg = &msg_control.cmsg;
> > +        cmsg->cmsg_len = CMSG_LEN(sizeof(passed_fd));
> > +        cmsg->cmsg_level = SOL_SOCKET;
> > +        cmsg->cmsg_type = SCM_RIGHTS;
> > +        memcpy(CMSG_DATA(cmsg), &passed_fd, sizeof(passed_fd));
> > +
> > +    }
> > +
> > +    do {
> > +        retval = sendmsg(sockfd, &msg, 0);
> > +    } while (retval < 0 && errno == EINTR);
> > +
> > +    return retval;
> > +}
> > +
> > +ssize_t qemu_recv_with_fd(int sockfd, int *passed_fd,
> > +                          void *buf, size_t len)
> > +{
> > +    struct iovec iov;
> > +    struct msghdr msg;
> > +    struct cmsghdr *cmsg;
> > +    union MsgControl msg_control;
> > +    int retval;
> > +    int data = *(int *)buf;
> > +
> > +    iov.iov_base = buf;
> > +    iov.iov_len = len;
> > +
> > +    memset(&msg, 0, sizeof(msg));
> > +    msg.msg_iov = &iov;
> > +    msg.msg_iovlen = 1;
> > +    msg.msg_control = &msg_control;
> > +    msg.msg_controllen = sizeof(msg_control);
> > +
> > +    do {
> > +        retval = recvmsg(sockfd, &msg, 0);
> > +    } while (retval < 0 && errno == EINTR);
> > +
> > +    if (retval <= 0) {
> > +        return retval;
> > +    }
> > +
> > +    if (data != *(int *)buf) {
> > +        *passed_fd = data;
> > +        return 0;
> > +    }
> Again cast issues
>
> > +
> > +    for (cmsg = CMSG_FIRSTHDR(&msg); cmsg; cmsg = CMSG_NXTHDR(&msg, cmsg)) {
> > +        if (cmsg->cmsg_len != CMSG_LEN(sizeof(int)) ||
> > +            cmsg->cmsg_level != SOL_SOCKET ||
> > +            cmsg->cmsg_type != SCM_RIGHTS) {
> > +            continue;
> > +        }
> > +
> > +        memcpy(passed_fd, CMSG_DATA(cmsg), sizeof(*passed_fd));
> > +        return 0;
> > +    }
> > +
> > +    *passed_fd = -ENFILE;
> > +    return retval;
> > +}
> > --
> Regards,
> Daniel


--
Lei