Module: xenomai-3
Branch: next
Commit: 6150e5b86ec32e1b1e41bbfbc9e3bbb8d619b08b
URL:
http://git.xenomai.org/?p=xenomai-3.git;a=commit;h=6150e5b86ec32e1b1e41bbfbc9e3bbb8d619b08b
Author: Philippe Gerum <r...@xenomai.org>
Date: Tue Feb 9 11:51:00 2016 +0100
copperplate/syncobj: fix race in aborted wait_grant/drain operations
Fixup a potential race upon return from grant/drain_wait operations,
e.g. given two threads A and B:
A:enqueue_waiter(self)
A:monitor_wait
A:monitor_unlock
A:[timed] sleep
A:wakeup on timeout/interrupt
B:monitor_lock
B:look_for_queued_waiter
(found A, update A's state)
B:monitor_unlock
A:dequeue_waiter(self)
A:return -ETIMEDOUT/-EINTR
The race may happen anytime between the timeout/interrupt event is
received by A, and the moment it grabs back the monitor lock before
unqueuing. When the race happens, B can squeeze in a signal before A
unqueues after resumption on error.
Problem: A's internal state has been updated (e.g. some data
transferred to it), but it will receive -ETIMEDOUT/-EINTR, causing it
to miss the update eventually.
The fix involves filtering out -ETIMEDOUT/-EINTR errors upon return
from wait_grant/drain operations whenever the syncobj was actually
signaled.
This issue was detected and described by
http://xenomai.org/pipermail/xenomai/2016-February/035852.html
---
lib/copperplate/syncobj.c | 51 ++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 46 insertions(+), 5 deletions(-)
diff --git a/lib/copperplate/syncobj.c b/lib/copperplate/syncobj.c
index 1b3d545..bf04b29 100644
--- a/lib/copperplate/syncobj.c
+++ b/lib/copperplate/syncobj.c
@@ -453,14 +453,55 @@ struct threadobj *syncobj_peek_drain(struct syncobj *sobj)
static int wait_epilogue(struct syncobj *sobj,
struct syncstate *syns,
- struct threadobj *current)
+ struct threadobj *current,
+ int ret)
{
current->run_state = __THREAD_S_RUNNING;
+ /*
+ * Fixup a potential race upon return from grant/drain_wait
+ * operations, e.g. given two threads A and B:
+ *
+ * A:enqueue_waiter(self)
+ * A:monitor_wait
+ * A:monitor_unlock
+ * A:[timed] sleep
+ * A:wakeup on timeout/interrupt
+ * B:monitor_lock
+ * B:look_for_queued_waiter
+ * (found A, update A's state)
+ * B:monitor_unlock
+ * A:dequeue_waiter(self)
+ * A:return -ETIMEDOUT/-EINTR
+ *
+ * The race may happen anytime between the timeout/interrupt
+ * event is received by A, and the moment it grabs back the
+ * monitor lock before unqueuing. When the race happens, B can
+ * squeeze in a signal before A unqueues after resumption on
+ * error.
+ *
+ * Problem: A's internal state has been updated (e.g. some
+ * data transferred to it), but it will receive
+ * -ETIMEDOUT/-EINTR, causing it to miss the update
+ * eventually.
+ *
+ * Solution: fixup the status code upon return from
+ * wait_grant/drain operations, so that -ETIMEDOUT/-EINTR is
+ * never returned to the caller if the syncobj was actually
+ * signaled. We still allow the SYNCOBJ_FLUSHED condition to
+ * override that success code though.
+ *
+ * Whether a condition should be deemed satisfied if it is
+ * signaled during the race window described above is
+ * debatable, but this is a simple and straightforward way to
+ * handle such grey area.
+ */
+
if (current->wait_sobj) {
dequeue_waiter(sobj, current);
current->wait_sobj = NULL;
- }
+ } else if (ret == -ETIMEDOUT || ret == -EINTR)
+ ret = 0;
sobj->wait_count--;
assert(sobj->wait_count >= 0);
@@ -477,7 +518,7 @@ static int wait_epilogue(struct syncobj *sobj,
if (current->wait_status & SYNCOBJ_FLUSHED)
return -EINTR;
- return 0;
+ return ret;
}
int syncobj_wait_grant(struct syncobj *sobj, const struct timespec *timeout,
@@ -515,7 +556,7 @@ int syncobj_wait_grant(struct syncobj *sobj, const struct
timespec *timeout,
pthread_setcancelstate(state, NULL);
- return wait_epilogue(sobj, syns, current) ?: ret;
+ return wait_epilogue(sobj, syns, current, ret);
}
int syncobj_wait_drain(struct syncobj *sobj, const struct timespec *timeout,
@@ -553,7 +594,7 @@ int syncobj_wait_drain(struct syncobj *sobj, const struct
timespec *timeout,
pthread_setcancelstate(state, NULL);
- return wait_epilogue(sobj, syns, current) ?: ret;
+ return wait_epilogue(sobj, syns, current, ret);
}
int syncobj_destroy(struct syncobj *sobj, struct syncstate *syns)
_______________________________________________
Xenomai-git mailing list
Xenomai-git@xenomai.org
http://xenomai.org/mailman/listinfo/xenomai-git
