mirrors/qemu
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

linux-user: Don't assume 0 is not a valid host timer_t value

Browse Source 
For handling guest POSIX timers, we currently use an array
g_posix_timers[], whose entries are a host timer_t value, or 0 for
"this slot is unused".  When the guest calls the timer_create syscall
we look through the array for a slot containing 0, and use that for
the new timer.

This scheme assumes that host timer_t values can never be zero.  This
is unfortunately not a valid assumption -- for some host libc
versions, timer_t values are simply indexes starting at 0.  When
using this kind of host libc, the effect is that the first and second
timers end up sharing a slot, and so when the guest tries to operate
on the first timer it changes the second timer instead.

Rework the timer allocation code, so that:
 * the 'slot in use' indication uses a separate array from the
   host timer_t array
 * we grab the free slot atomically, to avoid races when multiple
   threads call timer_create simultaneously
 * releasing an allocated slot is abstracted out into a new
   free_host_timer_slot() function called in the correct places

This fixes:
 * problems on hosts where timer_t 0 is valid
 * the FIXME in next_free_host_timer() about locking
 * bugs in the error paths in timer_create where we forgot to release
   the slot we grabbed, or forgot to free the host timer

Reported-by: Jon Alduan <jon.alduan@gmail.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20220725110035.1273441-1-peter.maydell@linaro.org>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
This commit is contained in:
Peter Maydell 2022-07-25 12:00:35 +01:00 committed by Laurent Vivier
parent 9b9145f04d
commit 9e59899f8c
1 changed files with 16 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
linux-user/syscall.c
View File

@ -525,20 +525,25 @@ _syscall4(int, sys_prlimit64, pid_t, pid, int, resource,
#if defined(TARGET_NR_timer_create)
/* Maximum of 32 active POSIX timers allowed at any one time. */
static timer_t g_posix_timers[32] = { 0, } ;
#define GUEST_TIMER_MAX 32
static timer_t g_posix_timers[GUEST_TIMER_MAX];
static int g_posix_timer_allocated[GUEST_TIMER_MAX];
static inline int next_free_host_timer(void)
{
int k ;
/* FIXME: Does finding the next free slot require a lock? */
for (k = 0; k < ARRAY_SIZE(g_posix_timers); k++) {
if (g_posix_timers[k] == 0) {
g_posix_timers[k] = (timer_t) 1;
int k;
for (k = 0; k < ARRAY_SIZE(g_posix_timer_allocated); k++) {
if (qatomic_xchg(g_posix_timer_allocated + k, 1) == 0) {
return k;
}
}
return -1;
}
static inline void free_host_timer_slot(int id)
{
qatomic_store_release(g_posix_timer_allocated + id, 0);
}
#endif
static inline int host_to_target_errno(int host_errno)
@ -12896,15 +12901,18 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
phost_sevp = &host_sevp;
ret = target_to_host_sigevent(phost_sevp, arg2);
if (ret != 0) {
free_host_timer_slot(timer_index);
return ret;
}
}
ret = get_errno(timer_create(clkid, phost_sevp, phtimer));
if (ret) {
phtimer = NULL;
free_host_timer_slot(timer_index);
} else {
if (put_user(TIMER_MAGIC | timer_index, arg3, target_timer_t)) {
timer_delete(*phtimer);
free_host_timer_slot(timer_index);
return -TARGET_EFAULT;
}
}
@ -13040,7 +13048,7 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
} else {
timer_t htimer = g_posix_timers[timerid];
ret = get_errno(timer_delete(htimer));
g_posix_timers[timerid] = 0;
free_host_timer_slot(timerid);
}
return ret;
}