From 8b1161ec8eaba3fa3f7d86e4a7b9a623a1bf2a01 Mon Sep 17 00:00:00 2001 From: James Prestwood Date: Mon, 1 Jun 2020 13:03:40 -0700 Subject: [PATCH] frame-xchg: fix bug when starting new xchg from callback This bug is caused by the following behavior: 1. Start a frame-xchg, wait for callback 2. From callback start a new frame-xchg, same prefix. The new frame-xchg request will detect that there is a duplicate watch, which is correct behavior. It will then remove this duplicate from the watchlist. The issue here is that we are in the watchlist notify loop from the original xchg. This causes that loop to read from the now freed watchlist item, causing an invalid read. Instead of freeing the item immediately, check if the notify loop is in progress and only set 'id' to zero and 'stale_items' to true. This will allow the notify loop to finish, then the watchlist code will prune out any stale items. If not in the notify loop the item can be freed as it was before. --- src/frame-xchg.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/frame-xchg.c b/src/frame-xchg.c index f713c07e..7e516d0a 100644 --- a/src/frame-xchg.c +++ b/src/frame-xchg.c @@ -541,7 +541,17 @@ static bool frame_watch_check_duplicate(void *data, void *user_data) } drop: - /* Drop the existing watch as a duplicate of the new one */ + /* + * Drop the existing watch as a duplicate of the new one. If we are in + * the watchlist notify loop, just mark this item as stale and it will + * be cleaned up afterwards + */ + if (watch->group->watches.in_notify) { + super->id = 0; + watch->group->watches.stale_items = true; + return false; + } + frame_watch_free(&watch->super); return true; }