Re: [Qemu-devel] [PATCH] linux-user: Fix up timer id handling

[Peter Maydell]

On 10 November 2014 20:33, 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 simplifies the code around timer id creation by introducing a 
> proper
> target_timer_id typedef that is s32, just like Linux has it. It also changes 
> the
> magic offset to a value that makes all timer ids be positive.
>
> Reported-by: Tom Musta <address@hidden>
> Signed-off-by: Alexander Graf <address@hidden>

> @@ -9638,12 +9658,12 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
> arg1,
>      case TARGET_NR_timer_gettime:
>      {
>          /* args: timer_t timerid, struct itimerspec *curr_value */
> -        target_ulong timerid = arg1;
> +        target_timer_t timerid = get_timer_id(arg1);
>
> -        if (!arg2) {
> -            return -TARGET_EFAULT;
> -        } else if (timerid >= ARRAY_SIZE(g_posix_timers)) {
> -            ret = -TARGET_EINVAL;
> +        if (timerid < 0) {
> +            ret = timerid;
> +        } else if (!arg2) {
> +            ret = -TARGET_EFAULT;

This is changing the order of the checks so that we prefer EINVAL
over EFAULT if the caller passes in NULL arg2 and a bad ID; this
is in fact in accordance with what the kernel does, so it's correct.

Reviewed-by: Peter Maydell <address@hidden>

thanks
-- PMM