contrib/plugins: Use GRWLock in execlog

execlog had the following comment:
> As we could have multiple threads trying to do this we need to
> serialise the expansion under a lock. Threads accessing already
> created entries can continue without issue even if the ptr array
> gets reallocated during resize.

However, when the ptr array gets reallocated, the other threads may have
a stale reference to the old buffer. This results in use-after-free.

Use GRWLock to properly fix this issue.

Fixes: 3d7caf145e ("contrib/plugins: add execlog to log instruction execution and memory access")
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20230912224107.29669-5-akihiko.odaki@daynix.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20231009164104.369749-12-alex.bennee@linaro.org>
This commit is contained in:
Akihiko Odaki 2023-10-09 17:40:50 +01:00 committed by Alex Bennée
parent fb13735ab4
commit 1063693e1c

View File

@ -19,7 +19,7 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
/* Store last executed instruction on each vCPU as a GString */ /* Store last executed instruction on each vCPU as a GString */
static GPtrArray *last_exec; static GPtrArray *last_exec;
static GMutex expand_array_lock; static GRWLock expand_array_lock;
static GPtrArray *imatches; static GPtrArray *imatches;
static GArray *amatches; static GArray *amatches;
@ -28,18 +28,16 @@ static GArray *amatches;
* Expand last_exec array. * Expand last_exec array.
* *
* As we could have multiple threads trying to do this we need to * As we could have multiple threads trying to do this we need to
* serialise the expansion under a lock. Threads accessing already * serialise the expansion under a lock.
* created entries can continue without issue even if the ptr array
* gets reallocated during resize.
*/ */
static void expand_last_exec(int cpu_index) static void expand_last_exec(int cpu_index)
{ {
g_mutex_lock(&expand_array_lock); g_rw_lock_writer_lock(&expand_array_lock);
while (cpu_index >= last_exec->len) { while (cpu_index >= last_exec->len) {
GString *s = g_string_new(NULL); GString *s = g_string_new(NULL);
g_ptr_array_add(last_exec, s); g_ptr_array_add(last_exec, s);
} }
g_mutex_unlock(&expand_array_lock); g_rw_lock_writer_unlock(&expand_array_lock);
} }
/** /**
@ -51,8 +49,10 @@ static void vcpu_mem(unsigned int cpu_index, qemu_plugin_meminfo_t info,
GString *s; GString *s;
/* Find vCPU in array */ /* Find vCPU in array */
g_rw_lock_reader_lock(&expand_array_lock);
g_assert(cpu_index < last_exec->len); g_assert(cpu_index < last_exec->len);
s = g_ptr_array_index(last_exec, cpu_index); s = g_ptr_array_index(last_exec, cpu_index);
g_rw_lock_reader_unlock(&expand_array_lock);
/* Indicate type of memory access */ /* Indicate type of memory access */
if (qemu_plugin_mem_is_store(info)) { if (qemu_plugin_mem_is_store(info)) {
@ -80,10 +80,14 @@ static void vcpu_insn_exec(unsigned int cpu_index, void *udata)
GString *s; GString *s;
/* Find or create vCPU in array */ /* Find or create vCPU in array */
g_rw_lock_reader_lock(&expand_array_lock);
if (cpu_index >= last_exec->len) { if (cpu_index >= last_exec->len) {
g_rw_lock_reader_unlock(&expand_array_lock);
expand_last_exec(cpu_index); expand_last_exec(cpu_index);
g_rw_lock_reader_lock(&expand_array_lock);
} }
s = g_ptr_array_index(last_exec, cpu_index); s = g_ptr_array_index(last_exec, cpu_index);
g_rw_lock_reader_unlock(&expand_array_lock);
/* Print previous instruction in cache */ /* Print previous instruction in cache */
if (s->len) { if (s->len) {