BUG/MINOR: hlua_fcn: fix potential UAF with Queue:pop_wait()

If Queue:pop_wait() excecuted from a stream context and pop_wait() is
aborted due to a Lua or ressource error, then the waiting object pointing
to the task will still be registered, so if the task eventually dissapears,
Queue:push() may try to wake invalid task pointer..

To prevent this bug from happening, we now rely on notification_* API to
deliver waiting signals. This way signals are properly garbage collected
when a lua context is destroyed.

It should be backported in 2.8 with 86fb22c55 ("MINOR: hlua_fcn: add Queue
class").
This patch depends on ("MINOR: task: add thread safe notification_new and
notification_wake variants")

(cherry picked from commit c6fa061f22e0409a9c1e0dbe9d4bd9a30eff6ba1)
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
(cherry picked from commit 51de928f9eda86631ef627d2a750a02857ccc38b)
[ada: ctx adjt]
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>

diff --git a/src/hlua_fcn.c b/src/hlua_fcn.c
index 0340ce1..79f4c28 100644 (file)
--- a/src/hlua_fcn.c
+++ b/src/hlua_fcn.c
@@ -513,22 +513,10 @@ struct hlua_queue_item {
        struct mt_list list;
 };
 
-/* used to store wait entries in queue->wait_tasks */
-struct hlua_queue_wait
-{
-       struct task *task;
-       struct mt_list entry;
-};
-
 /* This is the memory pool containing struct hlua_queue_item (queue items)
  */
 DECLARE_STATIC_POOL(pool_head_hlua_queue, "hlua_queue", sizeof(struct hlua_queue_item));
 
-/* This is the memory pool containing struct hlua_queue_wait
- * (queue waiting tasks)
- */
-DECLARE_STATIC_POOL(pool_head_hlua_queuew, "hlua_queuew", sizeof(struct hlua_queue_wait));
-
 static struct hlua_queue *hlua_check_queue(lua_State *L, int ud)
 {
        return hlua_checkudata(L, ud, class_queue_ref);
@@ -555,8 +543,6 @@ static int hlua_queue_push(lua_State *L)
 {
        struct hlua_queue *queue = hlua_check_queue(L, 1);
        struct hlua_queue_item *item;
-       struct mt_list *elt1, elt2;
-       struct hlua_queue_wait *waiter;
 
        if (lua_gettop(L) != 2 || lua_isnoneornil(L, 2)) {
                luaL_error(L, "unexpected argument");
@@ -581,9 +567,7 @@ static int hlua_queue_push(lua_State *L)
        MT_LIST_APPEND(&queue->list, &item->list);
 
        /* notify tasks waiting on queue:pop_wait() (if any) */
-       mt_list_for_each_entry_safe(waiter, &queue->wait_tasks, entry, elt1, elt2) {
-               task_wakeup(waiter->task, TASK_WOKEN_MSG);
-       }
+       notification_wake_mt(&queue->wait_tasks);
 
        lua_pushboolean(L, 1);
        return 1;
@@ -639,24 +623,25 @@ static int hlua_queue_pop(lua_State *L)
 static int _hlua_queue_pop_wait(lua_State *L, int status, lua_KContext ctx)
 {
        struct hlua_queue *queue = hlua_check_queue(L, 1);
-       struct hlua_queue_wait *wait = lua_touserdata(L, 2);
 
        /* new pop attempt */
        if (!_hlua_queue_pop(L, queue)) {
+               struct hlua *hlua;
+
+               hlua = hlua_gethlua(L);
+               if (!notification_new_mt(&hlua->com, &queue->wait_tasks, hlua->task)) {
+                       lua_pushnil(L);
+                       return 1; /* memory error, return nil */
+               }
                hlua_yieldk(L, 0, 0, _hlua_queue_pop_wait, TICK_ETERNITY, 0); // wait retry
                return 0; // never reached, yieldk won't return
        }
 
-       /* remove task from waiting list */
-       MT_LIST_DELETE(&wait->entry);
-       pool_free(pool_head_hlua_queuew, wait);
-
        return 1; // success
 }
 static int hlua_queue_pop_wait(lua_State *L)
 {
        struct hlua_queue *queue = hlua_check_queue(L, 1);
-       struct hlua_queue_wait *wait;
        struct hlua *hlua;
 
        BUG_ON(!queue);
@@ -676,21 +661,6 @@ static int hlua_queue_pop_wait(lua_State *L)
 
        /* no pending items, waiting required */
 
-       wait = pool_alloc(pool_head_hlua_queuew);
-       if (!wait) {
-               lua_pushnil(L);
-               return 1; /* memory error, return nil */
-       }
-
-       wait->task = hlua->task;
-       MT_LIST_INIT(&wait->entry);
-
-       /* add task to queue's wait list */
-       MT_LIST_TRY_APPEND(&queue->wait_tasks, &wait->entry);
-
-       /* push wait entry at index 2 on the stack (queue is already there) */
-       lua_pushlightuserdata(L, wait);
-
        /* Go to waiting loop which immediately performs a new attempt to make
         * sure we didn't miss a push during the wait entry initialization.
         *
@@ -736,20 +706,8 @@ static int hlua_queue_new(lua_State *L)
 static int hlua_queue_gc(struct lua_State *L)
 {
        struct hlua_queue *queue = hlua_check_queue(L, 1);
-       struct hlua_queue_wait *wait;
        struct hlua_queue_item *item;
 
-       /* Purge waiting tasks (if any)
-        *
-        * It is normally not expected to have waiting tasks, except if such
-        * task has been aborted while in the middle of a queue:pop_wait()
-        * function call.
-        */
-       while ((wait = MT_LIST_POP(&queue->wait_tasks, typeof(wait), entry))) {
-               /* free the wait entry */
-               pool_free(pool_head_hlua_queuew, wait);
-       }
-
        /* purge remaining (unconsumed) items in the queue */
        while ((item = MT_LIST_POP(&queue->list, typeof(item), list))) {
                /* free the queue item */