vhost: clear out unused SCM_RIGHTS file descriptors
authorStefan Hajnoczi <stefanha@redhat.com>
Mon, 5 Feb 2018 12:16:00 +0000 (13:16 +0100)
committerFerruh Yigit <ferruh.yigit@intel.com>
Fri, 30 Mar 2018 12:08:42 +0000 (14:08 +0200)
The number of file descriptors received is not stored by vhost_user.c.
vhost_user_set_mem_table() assumes that memory.nregions matches the
number of file descriptors received, but nothing guarantees this:

  for (i = 0; i < memory.nregions; i++)
      close(pmsg->fds[i]);

Another questionable code snippet is:

  case VHOST_USER_SET_LOG_FD:
      close(msg.fds[0]);

If not enough file descriptors were received then fds[] contains
uninitialized data from the stack (see read_fd_message()).  This might
cause non-vhost file descriptors to be closed if the uninitialized data
happens to match.

Refactoring vhost_user.c to pass around and check the number of file
descriptors everywhere would make the code more complex.  It is simpler
for read_fd_message() to set unused elements in fds[] to -1.  This way
close(-1) is called and no harm is done.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
lib/librte_vhost/socket.c

index 83befdc..776336c 100644 (file)
@@ -97,6 +97,7 @@ read_fd_message(int sockfd, char *buf, int buflen, int *fds, int fd_num)
        size_t fdsize = fd_num * sizeof(int);
        char control[CMSG_SPACE(fdsize)];
        struct cmsghdr *cmsg;
+       int got_fds = 0;
        int ret;
 
        memset(&msgh, 0, sizeof(msgh));
@@ -123,11 +124,16 @@ read_fd_message(int sockfd, char *buf, int buflen, int *fds, int fd_num)
                cmsg = CMSG_NXTHDR(&msgh, cmsg)) {
                if ((cmsg->cmsg_level == SOL_SOCKET) &&
                        (cmsg->cmsg_type == SCM_RIGHTS)) {
-                       memcpy(fds, CMSG_DATA(cmsg), fdsize);
+                       got_fds = (cmsg->cmsg_len - CMSG_LEN(0)) / sizeof(int);
+                       memcpy(fds, CMSG_DATA(cmsg), got_fds * sizeof(int));
                        break;
                }
        }
 
+       /* Clear out unused file descriptors */
+       while (got_fds < fd_num)
+               fds[got_fds++] = -1;
+
        return ret;
 }