On 10 November 2014 16:46, Alexander Graf <ag...@suse.de> wrote:
> When creating a timer handle, we give the timer id a special magic offset
> of 0xcafe0000. However, we never mask that offset out of the timer id before
> we start using it to dereference our timer array. So we always end up aborting
> timer operations because the timer id is out of bounds.
>
> This was not an issue before my patch e52a99f756e ("linux-user: Simplify
> timerid checks on g_posix_timers range") because before we would blindly mask
> anything above the first 16 bits.
>
> This patch is superior to the plain masking in that it also adds validity
> checks
> against the timer id to ensure we're always dealing with an actual timer id
> created by QEMU.
The commit message says this...
> @@ -9619,6 +9622,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
> arg1,
> * struct itimerspec * old_value */
> target_ulong timerid = arg1;
>
> + /* Convert QEMU provided timer ID back to internal 16bit index
> format */
> + if ((timerid & TIMER_MAGIC_MASK) == TIMER_MAGIC) {
> + timerid &= 0xffff;
> + }
...but the code doesn't actually fail EINVAL in the case that the
magic value doesn't match, so if you just pass in a small integer
for the timerid that will work even though the magic is wrong.
(You can't actually make it overflow the array this way, obviously.)
> +
> if (arg3 == 0 || timerid >= ARRAY_SIZE(g_posix_timers)) {
> ret = -TARGET_EINVAL;
> } else {
> @@ -9640,6 +9648,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
> arg1,
> /* args: timer_t timerid, struct itimerspec *curr_value */
> target_ulong timerid = arg1;
>
> + /* Convert QEMU provided timer ID back to internal 16bit index
> format */
> + if ((timerid & TIMER_MAGIC_MASK) == TIMER_MAGIC) {
> + timerid &= 0xffff;
> + }
> +
Same here.
thanks
- PMM
