|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-changelog] [xen stable-4.2] libxl: poll: Avoid fd deregistration race POLLNVAL crash
commit 32894093072c6ac5e680541669b0d1db263745fa
Author: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
AuthorDate: Wed Aug 12 12:50:12 2015 +0100
Commit: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
CommitDate: Wed Aug 12 13:09:55 2015 +0100
libxl: poll: Avoid fd deregistration race POLLNVAL crash
It can happen that an fd is deregistered, and closed, and then a new
fd opened, and reregistered, all while another thread is in poll().
If this happens poll might report POLLNVAL, but the event loop would
think that the fd was supposed to have been valid, and then fail an
assertion:
libxl_event.c:1183: afterpoll_check_fd: Assertion `poller->fds_changed ||
!(fds[slot].revents & 0x020)' failed.
We can't simply ignore POLLNVAL because if we have bugs which cause
messed-up fds, it is a serious problem which we really need to detect.
Instead, add extra tracking to spot when this possibility arises, and
abort on POLLNVAL if we are sure that it is unexpected.
Reported-by: Jim Fehlig <jfehlig@xxxxxxxx>
Signed-off-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
CC: Jim Fehlig <jfehlig@xxxxxxxx>
Acked-by: Wei Liu <wei.liu2@xxxxxxxxxx>
Tested-by: Jim Fehlig <jfehlig@xxxxxxxx>
(cherry picked from commit 681ce1681622a46d111cfdc4fc07e4cb565ae131)
(cherry picked from commit 7f7642f778b78e8e204fc082ce03072bb26887c7)
And, adjusted for semantic conflict over CTX vs ctx.
Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
(cherry picked from commit 3646b134c1673f09c0a239de10b0da4c9265c8e8)
Conflicts:
tools/libxl/libxl_event.c
Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
(cherry picked from commit 3bcb2c062a02e3c45d3f87478d2cbe1a134d395c)
---
tools/libxl/libxl.c | 2 ++
tools/libxl/libxl_event.c | 12 +++++++++++-
tools/libxl/libxl_internal.h | 13 +++++++++++++
3 files changed, 26 insertions(+), 1 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 0281c4d..04b3f62 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -53,6 +53,7 @@ int libxl_ctx_alloc(libxl_ctx **pctx, int version,
ctx->poller_app = 0;
LIBXL_LIST_INIT(&ctx->pollers_event);
LIBXL_LIST_INIT(&ctx->pollers_idle);
+ LIBXL_LIST_INIT(&ctx->pollers_fds_changed);
LIBXL_LIST_INIT(&ctx->efds);
LIBXL_TAILQ_INIT(&ctx->etimes);
@@ -167,6 +168,7 @@ int libxl_ctx_free(libxl_ctx *ctx)
libxl__poller_put(ctx, ctx->poller_app);
ctx->poller_app = NULL;
assert(LIBXL_LIST_EMPTY(&ctx->pollers_event));
+ assert(LIBXL_LIST_EMPTY(&ctx->pollers_fds_changed));
libxl__poller *poller, *poller_tmp;
LIBXL_LIST_FOREACH_SAFE(poller, &ctx->pollers_idle, entry, poller_tmp) {
libxl__poller_dispose(poller);
diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c
index 3abcfb5..21ded9a 100644
--- a/tools/libxl/libxl_event.c
+++ b/tools/libxl/libxl_event.c
@@ -220,6 +220,7 @@ int libxl__ev_fd_modify(libxl__gc *gc, libxl__ev_fd *ev,
short events)
void libxl__ev_fd_deregister(libxl__gc *gc, libxl__ev_fd *ev)
{
CTX_LOCK;
+ libxl__poller *poller;
if (!libxl__ev_fd_isregistered(ev)) {
DBG("ev_fd=%p deregister unregistered",ev);
@@ -232,6 +233,9 @@ void libxl__ev_fd_deregister(libxl__gc *gc, libxl__ev_fd
*ev)
LIBXL_LIST_REMOVE(ev, entry);
ev->fd = -1;
+ LIBXL_LIST_FOREACH(poller, &CTX->pollers_fds_changed, fds_changed_entry)
+ poller->fds_changed = 1;
+
out:
CTX_UNLOCK;
}
@@ -847,6 +851,8 @@ static int beforepoll_internal(libxl__gc *gc, libxl__poller
*poller,
*nfds_io = used;
+ poller->fds_changed = 0;
+
libxl__ev_time *etime = LIBXL_TAILQ_FIRST(&CTX->etimes);
if (etime) {
int our_timeout;
@@ -911,7 +917,7 @@ static int afterpoll_check_fd(libxl__poller *poller,
/* again, stale slot entry */
continue;
- assert(!(fds[slot].revents & POLLNVAL));
+ assert(poller->fds_changed || !(fds[slot].revents & POLLNVAL));
/* we mask in case requested events have changed */
int slot_revents = fds[slot].revents & events;
@@ -1291,6 +1297,7 @@ int libxl__poller_init(libxl_ctx *ctx, libxl__poller *p)
int r, rc;
p->fd_polls = 0;
p->fd_rindices = 0;
+ p->fds_changed = 0;
r = pipe(p->wakeup_pipe);
if (r) {
@@ -1343,12 +1350,15 @@ libxl__poller *libxl__poller_get(libxl_ctx *ctx)
}
}
+ LIBXL_LIST_INSERT_HEAD(&ctx->pollers_fds_changed, p,
+ fds_changed_entry);
return p;
}
void libxl__poller_put(libxl_ctx *ctx, libxl__poller *p)
{
if (!p) return;
+ LIBXL_LIST_REMOVE(p, fds_changed_entry);
LIBXL_LIST_INSERT_HEAD(&ctx->pollers_idle, p, entry);
}
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 5e2f8ea..636c91b 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -289,6 +289,18 @@ struct libxl__poller {
int (*fd_rindices)[3]; /* see libxl_event.c:beforepoll_internal */
int wakeup_pipe[2]; /* 0 means no fd allocated */
+
+ /*
+ * We also use the poller to record whether any fds have been
+ * deregistered since we entered poll. Each poller which is not
+ * idle is on the list pollers_fds_changed. fds_changed is
+ * cleared by beforepoll, and tested by afterpoll. Whenever an fd
+ * event is deregistered, we set the fds_changed of all non-idle
+ * pollers. So afterpoll can tell whether any POLLNVAL is
+ * plausibly due to an fd being closed and reopened.
+ */
+ LIBXL_LIST_ENTRY(libxl__poller) fds_changed_entry;
+ bool fds_changed;
};
struct libxl__gc {
@@ -330,6 +342,7 @@ struct libxl__ctx {
libxl__poller *poller_app; /* libxl_osevent_beforepoll and _afterpoll */
LIBXL_LIST_HEAD(, libxl__poller) pollers_event, pollers_idle;
+ LIBXL_LIST_HEAD(, libxl__poller) pollers_fds_changed;
LIBXL_SLIST_HEAD(libxl__osevent_hook_nexi, libxl__osevent_hook_nexus)
hook_fd_nexi_idle, hook_timeout_nexi_idle;
--
generated by git-patchbot for /home/xen/git/xen.git#stable-4.2
_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |