qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Lei Li
Subject: Re: [Qemu-devel] [PATCH 1/6] qemu-fd-exchange: provide common methods for exchange fd
Date: Fri, 17 Jan 2014 11:41:03 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/17.0 Thunderbird/17.0

On 01/16/2014 11:26 PM, Eric Blake wrote:
On 01/08/2014 02:12 AM, 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
s/Internel/Internal/


+++ b/util/qemu-fd-exchange.c
@@ -0,0 +1,97 @@
+/*
+ * Internel common methods for exchange of FD
and again.

Good catch! Thanks.


+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;
Is it safe to assume that buf is aligned well enough to be casting it to
int* then dereferencing it?  Why not just type the parameter correctly

That's because there would be different type for this parameter.

to begin with?  And why are you even writing into the caller's buffer
when they pass a negative fd, but leaving it alone when they pass a
non-negative fd?

That's just the original logical of exchange fd for proxy fs driver,

    if (fd < 0) {
        data = fd;
    } else {
        data = V9FS_FD_VALID;
        ....
    }

This common method don't leave it alone when a non-negative fd passed,
it'll be the same as the check value passed from the caller.

+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;
Again, why not type buf correctly, since otherwise you risk a user
passing in a buffer that is unsuitably aligned for dereferencing as an
int pointer.

+
+    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);
+
Should you take advantage of Linux' ability to use MSG_CMSG_CLOEXEC to
guarantee the received fd is atomically marked cloexec when possible?

Whether close the fd in the common method depends on the process
of these current users (they are not the same). It'd be better to
let the users handling the close of fd to fit it.




+    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;
+    }
+
+    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;
+    }
And even when MSG_CMSG_CLOEXEC is not available, shouldn't you ensure
that cloexec is set after the fact?

That's a good suggestion, thanks.



--
Lei




reply via email to

[Prev in Thread] Current Thread [Next in Thread]