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

xenstored file descriptor leak

  • To: xen-devel@xxxxxxxxxxxxx
  • From: Manuel Bouyer <bouyer@xxxxxxxxxxxxxxx>
  • Date: Tue, 2 Feb 2021 19:37:35 +0100
  • Delivery-date: Tue, 02 Feb 2021 18:37:46 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

on NetBSD I'm tracking down an issue where xenstored never closes its
file descriptor to /var/run/xenstored/socket and instead loops at 100%
CPU on these descriptors.

xenstored loops because poll(2) returns a POLLIN event for these
descriptors but they are marked is_ignored = true. 

I'm seeing this with xen 4.15, 4.13 and has also been reported with 4.11
with latest security patches.
It seems to have started with patches for XSA-115 (A user reported this
for 4.11)

I've tracked it down to a difference in poll(2) implementation; it seems
that linux will return something that is not (POLLIN|POLLOUT) when a
socket if closed; while NetBSD returns POLLIN (this matches the NetBSD's
man page).

First I think there may be a security issue here, as, even on linux it should
be possible for a client to cause a socket to go to the is_ignored state,
while still sending data and cause xenstored to go to a 100% CPU loop.
I think this is needed anyway:

--- xenstored_core.c.orig       2021-02-02 18:06:33.389316841 +0100
+++ xenstored_core.c    2021-02-02 19:27:43.761877371 +0100
@@ -397,9 +397,12 @@
                                *ptimeout = 0;
                } else {
-                       short events = POLLIN|POLLPRI;
-                       if (!list_empty(&conn->out_list))
-                               events |= POLLOUT;
+                       short events = 0;
+                       if (!conn->is_ignored) {
+                               events |= POLLIN|POLLPRI;
+                               if (!list_empty(&conn->out_list))
+                                       events |= POLLOUT;
+                       }
                        conn->pollfd_idx = set_fd(conn->fd, events);

Now I wonder if, on NetBSD at last, a read error or short read shouldn't
cause the socket to be closed, as with:

@@ -1561,6 +1565,8 @@
+       /* we don't want to keep this connection alive */
+       talloc_free(conn);
 static void handle_output(struct connection *conn)

what do you think ?

Manuel Bouyer <bouyer@xxxxxxxxxxxxxxx>
     NetBSD: 26 ans d'experience feront toujours la difference



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.