[PATCH BlueZ 1/2] shared: mainloop: Skip removed mainloop events

[PATCH BlueZ 1/2] shared: mainloop: Skip removed mainloop events

[Pauli Virtanen]

mainloop_remove_fd() on an fd that occurs in the same epoll_wait batch
as the current callback, causes ASan crash.

Several callsites do this, crash is sporadic since it requires suitable
timing.

Fix by adding registration ID in each event and dispatching only if that
registration is still active.

Fixes random btvirt crashes.

Log:
ERROR: AddressSanitizer: heap-use-after-free on address 0x7b7576be3f18
READ of size 8 at 0x7b7576be3f18 thread T0
    #0 0x5637f7b4014f in mainloop_run src/shared/mainloop.c:104
    #1 0x5637f7b41f79 in mainloop_run_with_signal src/shared/mainloop-notify.c:196
    #2 0x5637f7aa598b in main emulator/main.c:310
freed by thread T0 here:
    #0 0x7f45788ee4cf in free.part.0 (/lib64/libasan.so.8+0xee4cf)
    #1 0x5637f7b40d05 in mainloop_remove_fd src/shared/mainloop.c:213
previously allocated by thread T0 here:
    #0 0x7f45788ef24f in calloc (/lib64/libasan.so.8+0xef24f)
    #1 0x5637f7b4034b in mainloop_add_fd src/shared/mainloop.c:142

Assisted-by: opencode:gpt-6-sol
---

Notes:
    Triaged with LLM, patches cleaned up manually.

 src/shared/mainloop.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/src/shared/mainloop.c b/src/shared/mainloop.c
index 9a2e1eee6..b3ec0f2b1 100644
--- a/src/shared/mainloop.c
+++ b/src/shared/mainloop.c
@@ -35,9 +35,11 @@
 static int epoll_fd;
 static int epoll_terminate;
 static int exit_status = EXIT_SUCCESS;
+static uint64_t next_id;
 
 struct mainloop_data {
 	int fd;
+	uint64_t id;
 	uint32_t events;
 	mainloop_event_func callback;
 	mainloop_destroy_func destroy;
@@ -99,7 +101,13 @@ int mainloop_run(void)
 			continue;
 
 		for (n = 0; n < nfds; n++) {
-			struct mainloop_data *data = events[n].data.ptr;
+			uint64_t id = events[n].data.u64;
+			int fd = id % MAX_MAINLOOP_ENTRIES;
+			struct mainloop_data *data = mainloop_list[fd];
+
+			/* Another callback may have removed it */
+			if (!data || data->id != id)
+				continue;
 
 			data->callback(data->fd, events[n].events,
 							data->user_data);
@@ -145,6 +153,7 @@ int mainloop_add_fd(int fd, uint32_t events, mainloop_event_func callback,
 
 	memset(data, 0, sizeof(*data));
 	data->fd = fd;
+	data->id = ++next_id * MAX_MAINLOOP_ENTRIES + fd;
 	data->events = events;
 	data->callback = callback;
 	data->destroy = destroy;
@@ -152,7 +161,7 @@ int mainloop_add_fd(int fd, uint32_t events, mainloop_event_func callback,
 
 	memset(&ev, 0, sizeof(ev));
 	ev.events = events;
-	ev.data.ptr = data;
+	ev.data.u64 = data->id;
 
 	err = epoll_ctl(epoll_fd, EPOLL_CTL_ADD, data->fd, &ev);
 	if (err < 0) {
@@ -180,7 +189,7 @@ int mainloop_modify_fd(int fd, uint32_t events)
 
 	memset(&ev, 0, sizeof(ev));
 	ev.events = events;
-	ev.data.ptr = data;
+	ev.data.u64 = data->id;
 
 	err = epoll_ctl(epoll_fd, EPOLL_CTL_MOD, data->fd, &ev);
 	if (err < 0)
-- 
2.55.0

[PATCH BlueZ 2/2] unit: test-mainloop: add basic mainloop test

[Pauli Virtanen]

Exercise basic mainloop fd add / remove / modify and timeouts.

src/shared/tester.h depends on mainloop and cannot be used here, so
write tests without the framework.

Add tests:

Test Modify File Descriptor
Test Re-Add Other Callback
Test Timeout

Assisted-by: opencode:gpt-6-sol
---
 .gitignore           |   1 +
 Makefile.am          |   5 ++
 unit/test-mainloop.c | 171 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 177 insertions(+)
 create mode 100644 unit/test-mainloop.c

diff --git a/.gitignore b/.gitignore
index b1d63e1d4..a52e6eb1b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -97,6 +97,7 @@ tools/isotest
 tools/iso-tester
 test/bluezutils.pyc
 unit/test-tester
+unit/test-mainloop
 unit/test-ringbuf
 unit/test-queue
 unit/test-util
diff --git a/Makefile.am b/Makefile.am
index 1d46b9b93..53a3a15c3 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -593,6 +593,11 @@ unit_test_tester_SOURCES = unit/test-tester.c
 unit_test_tester_LDADD = src/libshared-glib.la lib/libbluetooth-internal.la \
 								$(GLIB_LIBS)
 
+unit_tests += unit/test-mainloop
+
+unit_test_mainloop_SOURCES = unit/test-mainloop.c
+unit_test_mainloop_LDADD = src/libshared-mainloop.la
+
 unit_tests += unit/test-eir
 
 unit_test_eir_SOURCES = unit/test-eir.c src/eir.c src/uuid-helper.c
diff --git a/unit/test-mainloop.c b/unit/test-mainloop.c
new file mode 100644
index 000000000..70ee03ee4
--- /dev/null
+++ b/unit/test-mainloop.c
@@ -0,0 +1,171 @@
+// SPDX-License-Identifier: LGPL-2.1-or-later
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2026 Pauli Virtanen
+ *
+ */
+
+#include <unistd.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+#include "src/shared/mainloop.h"
+
+
+#define test_failed_msg(msg) \
+	fail_and_abort(msg, __FILE__, __LINE__, __func__)
+
+#define test_failed() \
+	test_failed_msg("")
+
+#define check(condition) \
+	({ if (!(condition)) test_failed_msg(#condition " not true"); })
+
+
+struct test_data {
+	int fds[4];
+	unsigned int old_calls;
+	unsigned int new_calls;
+	unsigned int destroys;
+	unsigned int timeout_calls;
+};
+
+
+static void fail_and_abort(const char *msg, const char *file, int line,
+							const char *func)
+{
+	fprintf(stderr, "test failed\n");
+	fprintf(stderr, "    %s in %s:%d (%s)\n", msg, file, line, func);
+	abort();
+}
+
+static void teardown(struct test_data *data)
+{
+	int i;
+
+	for (i = 0; i < 4; ++i)
+		if (data->fds[i] > 0)
+			close(data->fds[i]);
+}
+
+static void destroy(void *user_data)
+{
+	struct test_data *data = user_data;
+
+	data->destroys++;
+}
+
+static void fd_callback(int fd, uint32_t events, void *user_data)
+{
+	struct test_data *data = user_data;
+	char buf;
+
+	check(events & EPOLLIN);
+	check(read(fd, &buf, 1) == 1);
+	check(buf == 'a');
+	data->old_calls++;
+	mainloop_exit_failure();
+}
+
+static void test_modify_fd(struct test_data *data)
+{
+	check(pipe(data->fds) == 0);
+
+	mainloop_init();
+
+	check(mainloop_add_fd(data->fds[0], EPOLLOUT, fd_callback,
+						data, destroy) == 0);
+	check(mainloop_modify_fd(data->fds[0], EPOLLIN) == 0);
+	check(write(data->fds[1], "a", 1) == 1);
+	check(mainloop_run() == EXIT_FAILURE);
+	check(data->old_calls == 1);
+	check(data->destroys == 1);
+}
+
+static void new_callback(int fd, uint32_t events, void *user_data)
+{
+	struct test_data *data = user_data;
+
+	data->new_calls++;
+}
+
+static void old_callback(int fd, uint32_t events, void *user_data)
+{
+	struct test_data *data = user_data;
+	int other_fd = fd == data->fds[0] ? data->fds[2] : data->fds[0];
+
+	data->old_calls++;
+	check(mainloop_remove_fd(other_fd) == 0);
+	check(data->destroys == 1);
+	check(mainloop_add_fd(other_fd, EPOLLIN, new_callback, data,
+								destroy) == 0);
+	mainloop_exit_success();
+}
+
+static void test_readd_other_callback(struct test_data *data)
+{
+	check(pipe(&data->fds[0]) == 0);
+	check(pipe(&data->fds[2]) == 0);
+
+	mainloop_init();
+
+	check(mainloop_add_fd(data->fds[0], EPOLLIN, old_callback,
+							data, destroy) == 0);
+	check(mainloop_add_fd(data->fds[2], EPOLLIN, old_callback,
+							data, destroy) == 0);
+	check(write(data->fds[1], "a", 1) == 1);
+	check(write(data->fds[3], "b", 1) == 1);
+
+	check(mainloop_run() == 0);
+
+	check(data->old_calls == 1);
+	check(data->new_calls == 0);
+	check(data->destroys == 3);
+}
+
+static void timeout_callback(int id, void *user_data)
+{
+	struct test_data *data = user_data;
+
+	data->timeout_calls++;
+	if (data->timeout_calls == 1) {
+		check(mainloop_modify_timeout(id, 1) == 0);
+		return;
+	}
+
+	check(mainloop_remove_timeout(id) == 0);
+	mainloop_quit();
+}
+
+static void test_timeout(struct test_data *data)
+{
+	int id;
+
+	mainloop_init();
+
+	id = mainloop_add_timeout(0, timeout_callback, data, destroy);
+	check(id > 0);
+	check(mainloop_modify_timeout(id, 1) == 0);
+	check(mainloop_run() == EXIT_SUCCESS);
+	check(data->timeout_calls == 2);
+	check(data->destroys == 1);
+}
+
+#define define_test(name, function)				\
+	do {							\
+		static struct test_data data = {};		\
+		fprintf(stderr, "%s - ", name);			\
+		function(&data);				\
+		fprintf(stderr, "test passed\n");		\
+		teardown(&data);				\
+	} while (0)
+
+int main(int argc, char *argv[])
+{
+	define_test("Test Modify File Descriptor", test_modify_fd);
+	define_test("Test Re-Add Other Callback", test_readd_other_callback);
+	define_test("Test Timeout", test_timeout);
+	return 0;
+}
-- 
2.55.0

RE: [BlueZ,1/2] shared: mainloop: Skip removed mainloop events

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 2273 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/series/1174578/

---Test result---

Test Summary:
CheckPatch                    FAIL      1.13 seconds
GitLint                       FAIL      0.68 seconds
BuildEll                      PASS      19.09 seconds
BluezMake                     PASS      381.66 seconds
MakeCheck                     PASS      14.33 seconds
MakeDistcheck                 PASS      137.22 seconds
CheckValgrind                 PASS      227.86 seconds
CheckSmatch                   PASS      284.89 seconds
bluezmakeextell               PASS      92.27 seconds
TestFunctional                PASS      1082.60 seconds
IncrementalBuild              PASS      406.78 seconds
ScanBuild                     PASS      1103.09 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[BlueZ,1/2] shared: mainloop: Skip removed mainloop events
WARNING:COMMIT_LOG_LONG_LINE: Prefer a maximum 75 chars per line (possible unwrapped commit description?)
#96: 
    #1 0x5637f7b41f79 in mainloop_run_with_signal src/shared/mainloop-notify.c:196

/home/runner/work/bluez/bluez/src/patch/14849970.patch total: 0 errors, 1 warnings, 48 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/home/runner/work/bluez/bluez/src/patch/14849970.patch has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[BlueZ,1/2] shared: mainloop: Skip removed mainloop events

18: B1 Line exceeds max length (82>80): "    #1 0x5637f7b41f79 in mainloop_run_with_signal src/shared/mainloop-notify.c:196"


https://github.com/bluez/bluez/pull/2597

---
Regards,
Linux Bluetooth

Re: [PATCH BlueZ 1/2] shared: mainloop: Skip removed mainloop events

[patchwork-bot+bluetooth]

Hello:

This series was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Sun, 27 Sep 2026 20:49:49 +0300 you wrote:
> mainloop_remove_fd() on an fd that occurs in the same epoll_wait batch
> as the current callback, causes ASan crash.
> 
> Several callsites do this, crash is sporadic since it requires suitable
> timing.
> 
> Fix by adding registration ID in each event and dispatching only if that
> registration is still active.
> 
> [...]

Here is the summary with links:
  - [BlueZ,1/2] shared: mainloop: Skip removed mainloop events
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c2ef55e49f2b
  - [BlueZ,2/2] unit: test-mainloop: add basic mainloop test
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=45c3b69db034

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html