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

[Lei Li <lilei@linux.vnet.ibm.com>]

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 <lilei@linux.vnet.ibm.com>
>> ---
>>   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