Re: [Qemu-devel] [PATCH 2.2 2/2] linux-user: Properly handle timer magic offset

[Peter Maydell]

On 10 November 2014 16:46, Alexander Graf <address@hidden> 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