diff options
| author | Erik Schilling <erik.schilling@linaro.org> | 2023-06-26 15:14:29 +0200 |
|---|---|---|
| committer | Bartosz Golaszewski <bartosz.golaszewski@linaro.org> | 2023-06-26 16:31:39 +0200 |
| commit | 370bf6825d2750b8328d0d1aeeb8075c2edb174c (patch) | |
| tree | cafa9332d9033fb047572cffcb959dbf81613a91 | |
| parent | 96c52d3e595be1ef0c247d53882abf8052d1a752 (diff) | |
| download | libgpiod-370bf6825d2750b8328d0d1aeeb8075c2edb174c.tar.gz | |
gpiosim: fix data race that corrupts heap
Hit this while seeing some heap corruptions when running cargo test on
the Rust bindings.
Took a bit to track down since I first used address sanitizers, but with
the thread sanitizer it becomes obvious immediately (output simplified):
==================
WARNING: ThreadSanitizer: data race (pid=288119)
Write of size 8 at 0x0000018f1e78 by thread T6:
#0 id_free /libgpiod/tests/gpiosim/gpiosim.c:141:17
#1 dev_release /libgpiod/tests/gpiosim/gpiosim.c:600:2
#2 refcount_dec /libgpiod/tests/gpiosim/gpiosim.c:176:3
#3 gpiosim_dev_unref /libgpiod/tests/gpiosim/gpiosim.c:671:2
#4 bank_release /libgpiod/tests/gpiosim/gpiosim.c:873:2
#5 refcount_dec /libgpiod/tests/gpiosim/gpiosim.c:176:3
#6 gpiosim_bank_unref /libgpiod/tests/gpiosim/gpiosim.c:941:2
[...]
Previous write of size 8 at 0x0000018f1e78 by thread T1:
#0 id_free /libgpiod/tests/gpiosim/gpiosim.c:141:17
#1 bank_release /libgpiod/tests/gpiosim/gpiosim.c:878:2
#2 refcount_dec /libgpiod/tests/gpiosim/gpiosim.c:176:3
#3 gpiosim_bank_unref /libgpiod/tests/gpiosim/gpiosim.c:941:2
[...]
Location is global 'id_del_ctx' of size 16 at 0x0000018f1e70
Thread T6 'chip::verify::f' (tid=288126, running) created by main thread at:
#7 test::run_tests::hd53a07a011bd771f /.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/test/src/lib.rs:407:21
[...]
Thread T1 'chip::open::gpi' (tid=288121, finished) created by main thread at:
#7 test::run_tests::hd53a07a011bd771f /.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/test/src/lib.rs:407:21
[...]
SUMMARY: ThreadSanitizer: data race /libgpiod/tests/gpiosim/gpiosim.c:141:17 in id_free
==================
This eventually can either lead to leaks or double free's that corrupt
the heap and lead to crashes.
The issue got introduced when a previously local variable that did not
require protection was turned into a global variable.
Fixes: 5e111df2fca5 ("gpiosim: use twalk() instead of twalk_r()")
Signed-off-by: Erik Schilling <erik.schilling@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
| -rw-r--r-- | tests/gpiosim/gpiosim.c | 4 |
1 files changed, 2 insertions, 2 deletions
diff --git a/tests/gpiosim/gpiosim.c b/tests/gpiosim/gpiosim.c index b49a61af..fca6b7fa 100644 --- a/tests/gpiosim/gpiosim.c +++ b/tests/gpiosim/gpiosim.c @@ -137,11 +137,11 @@ static int id_alloc(void) static void id_free(int id) { + pthread_mutex_lock(&id_lock); + id_del_ctx.id = id; id_del_ctx.idp = NULL; - pthread_mutex_lock(&id_lock); - twalk(id_root, id_del); if (id_del_ctx.idp) { tdelete(id_del_ctx.idp, &id_root, id_compare); |