Re: [Qemu-devel] [PATCH 2.2 v2 1/3] linux-user: Fix timer_create timer id return type

[Peter Maydell]

On 10 November 2014 17:44, Alexander Graf <address@hidden> wrote:
> The linux syscall to create_timer really gets an s32* parameter to store
> its timer id into, not an abi_ulong*. Remove the useless struct encapsulation
> and directly write the 32bit value into memory.

> @@ -9601,11 +9600,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
> arg1,
>              if (ret) {
>                  phtimer = NULL;
>              } else {
> -                if (!lock_user_struct(VERIFY_WRITE, ptarget_timer, arg3, 1)) 
> {
> +                if (put_user(0xcafe0000 | timer_index, arg3, 
> target_timer_t)) {
>                      goto efault;
>                  }
> -                ptarget_timer->ptr = tswap32(0xcafe0000 | timer_index);
> -                unlock_user_struct(ptarget_timer, arg3, 1);
>              }
>          }
>          break;
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index c9e6323..ebb3be1 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -2564,10 +2564,7 @@ struct target_ucred {
>
>  #endif
>
> -
> -struct target_timer_t {
> -    abi_ulong ptr;
> -};
> +typedef int32_t target_timer_t;

This means that all our valid timer_t values will be negative
(since the magic value we've picked happens to have bit 31 set),
whereas the kernel is careful to keep all its valid timer_t
values >= 0. That kind of worries me, since I could see userspace
code making signedness assumptions. Can we use a mask value
that doesn't set bit 31?

(It also makes it easier to reason about the bits of QEMU
code which take timer_t arguments; in particular notice that
        target_ulong timerid = arg1;
is likely to end up with 0xffffffffcafexxxx due to sign
extension on 64 bit guests.)

-- PMM