From: Waldemar Kozaczuk <[email protected]>
Committer: WALDEMAR KOZACZUK <[email protected]>
Branch: master

waitqueue: fix edge condition in disarm()

As the issue #1285 describes, following edge condition is not handled
correctly and results in a crash:

1. wo_older.arm() //(of wait_object<waitqueue> type)
2. wo_newer1.arm()
3. wo_older.disarm()
4. wo_newer2.arm() // Crash would happen here
5. wo_newer1.disarm()
6. wo_newer2.disarm()

./scripts/run.py -e '/tests/tst-wait-for.so'
OSv v0.57.0-109-gcf0f7526
eth0: 192.168.122.15
Booted up in 180.05 ms
Cmdline: /tests/tst-wait-for.so
Running 8 test cases...
page fault outside application, addr: 0x0000000000000000
[registers]
RIP: 0x00000000403a18c1 <sched::wait_object<waitqueue>::arm()+17>
RFL: 0x0000000000010206  CS:  0x0000000000000008  SS:  0x0000000000000010
RAX: 0x00002000001ff770  RBX: 0x00002000001ff7f0  RCX: 0x0000000000000000  RDX: 
0x0000000000000000
RSI: 0x0000600000fd4c00  RDI: 0x00002000001ff980  RBP: 0x00002000001ff9f0  R8:  
0x0000000000000000
R9:  0x0000000000000000  R10: 0x0000100000007692  R11: 0x0000000000000063  R12: 
0x00002000001ff7d0
R13: 0x0000000000000000  R14: 0x0000100000007692  R15: 0x0000000000000063  RSP: 
0x00002000001ff6b8
Aborted

[backtrace]
0x0000000040205287 <???+1075860103>
0x0000000040306b5b <page_fault+139>
0x0000000040305892 <???+1076910226>
0x00001000000083c2 <???+33730>

This patch adds new test cases to tst-wait-for.cc that validate arm()
and disarm(). The 3rd one - test_waitqueue_linked_list_3 - formulates
the exact scenario this patch fixes.

Fixes #1285

Signed-off-by: Waldemar Kozaczuk <[email protected]>

---
diff --git a/core/waitqueue.cc b/core/waitqueue.cc
--- a/core/waitqueue.cc
+++ b/core/waitqueue.cc
@@ -38,7 +38,7 @@ void wait_object<waitqueue>::disarm()
     while (*pnext) {
         if (&_wr == *pnext) {
             *pnext = _wr.next;
-            if (!*pnext || !(*pnext)->next) {
+            if (!*pnext) {
                 fifo.newest = newest;
             }
             break;
diff --git a/tests/tst-wait-for.cc b/tests/tst-wait-for.cc
--- a/tests/tst-wait-for.cc
+++ b/tests/tst-wait-for.cc
@@ -123,4 +123,52 @@ BOOST_AUTO_TEST_CASE(test_wait_for_predicate)
     false_waker->join();
 }
 
+BOOST_AUTO_TEST_CASE(test_waitqueue_linked_list_1)
+{
+    waitqueue wq;
+    mutex mtx;
+    sched::wait_object<waitqueue> wo(wq, &mtx);
+    wo.arm();
+    BOOST_REQUIRE(!wq.empty());
+    wo.disarm();
+    BOOST_REQUIRE(wq.empty());
+}
+
+BOOST_AUTO_TEST_CASE(test_waitqueue_linked_list_2)
+{
+    waitqueue wq;
+    mutex mtx;
+    sched::wait_object<waitqueue> wo_older(wq, &mtx);
+    wo_older.arm();
+    BOOST_REQUIRE(!wq.empty());
+    sched::wait_object<waitqueue> wo_newer(wq, &mtx);
+    wo_newer.arm();
+    BOOST_REQUIRE(!wq.empty());
+    wo_older.disarm();
+    BOOST_REQUIRE(!wq.empty());
+    wo_newer.disarm();
+    BOOST_REQUIRE(wq.empty());
+}
+
+BOOST_AUTO_TEST_CASE(test_waitqueue_linked_list_3)
+{
+    waitqueue wq;
+    mutex mtx;
+    sched::wait_object<waitqueue> wo_older(wq, &mtx);
+    wo_older.arm();
+    BOOST_REQUIRE(!wq.empty());
+    sched::wait_object<waitqueue> wo_newer_1(wq, &mtx);
+    wo_newer_1.arm();
+    BOOST_REQUIRE(!wq.empty());
+    wo_older.disarm();
+    BOOST_REQUIRE(!wq.empty());
+    sched::wait_object<waitqueue> wo_newer_2(wq, &mtx);
+    wo_newer_2.arm();
+    BOOST_REQUIRE(!wq.empty());
+    wo_newer_1.disarm();
+    BOOST_REQUIRE(!wq.empty());
+    wo_newer_2.disarm();
+    BOOST_REQUIRE(wq.empty());
+}
+
 OSV_ELF_MLOCK_OBJECT();

-- 
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/osv-dev/00000000000042bf6d060ce47f93%40google.com.

Reply via email to