[v2] linux-user: ioctl() command type is int

When executing a 64bit target chroot on 64bit host,
the ioctl() command can mismatch.

It seems the previous commit doesn't solve the problem in
my case:

	9c6bf9c7 linux-user: Fix ioctl cmd type mismatch on 64-bit targets

For example, a ppc64 chroot on an x86_64 host:

bash-4.3# ls
Unsupported ioctl: cmd=0x80087467
Unsupported ioctl: cmd=0x802c7415

The origin of the problem is in syscall.c:do_ioctl().

	static abi_long do_ioctl(int fd, abi_long cmd, abi_long arg)

In this case (ppc64) abi_long is long (on the x86_64), and

    cmd = 0x0000000080087467

then
	if (ie->target_cmd == cmd)

target_cmd is int, so target_cmd = 0x80087467
and to compare an int with a long, the sign is extended to 64bit,
so the comparison is:

	if (0xffffffff80087467 == 0x0000000080087467)

which doesn't match whereas it should.

This patch uses int in the case of the target command type
instead of abi_long (and for consistency, update IOCTLEntry).

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
v2: use int instead of abi_int, as it is recommended by Peter Maydell.

 linux-user/syscall.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

On 23 May 2015 at 14:17, Laurent Vivier <laurent@vivier.eu> wrote:
> When executing a 64bit target chroot on 64bit host,
> the ioctl() command can mismatch.
>
> It seems the previous commit doesn't solve the problem in
> my case:
>
>         9c6bf9c7 linux-user: Fix ioctl cmd type mismatch on 64-bit targets
>
> For example, a ppc64 chroot on an x86_64 host:
>
> bash-4.3# ls
> Unsupported ioctl: cmd=0x80087467
> Unsupported ioctl: cmd=0x802c7415
>
> The origin of the problem is in syscall.c:do_ioctl().
>
>         static abi_long do_ioctl(int fd, abi_long cmd, abi_long arg)
>
> In this case (ppc64) abi_long is long (on the x86_64), and
>
>     cmd = 0x0000000080087467
>
> then
>         if (ie->target_cmd == cmd)
>
> target_cmd is int, so target_cmd = 0x80087467
> and to compare an int with a long, the sign is extended to 64bit,
> so the comparison is:
>
>         if (0xffffffff80087467 == 0x0000000080087467)
>
> which doesn't match whereas it should.
>
> This patch uses int in the case of the target command type
> instead of abi_long (and for consistency, update IOCTLEntry).
>
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

On Saturday, May 23, 2015 4:17:05 PM EEST, Laurent Vivier wrote:
> When executing a 64bit target chroot on 64bit host,
> the ioctl() command can mismatch.
>
> It seems the previous commit doesn't solve the problem in
> my case:
>
> 	9c6bf9c7 linux-user: Fix ioctl cmd type mismatch on 64-bit targets
>
> For example, a ppc64 chroot on an x86_64 host:
>
> bash-4.3# ls
> Unsupported ioctl: cmd=0x80087467
> Unsupported ioctl: cmd=0x802c7415
>
> The origin of the problem is in syscall.c:do_ioctl().
>
> 	static abi_long do_ioctl(int fd, abi_long cmd, abi_long arg)
>
> In this case (ppc64) abi_long is long (on the x86_64), and
>
>     cmd = 0x0000000080087467
>
> then
> 	if (ie->target_cmd == cmd)
>
> target_cmd is int, so target_cmd = 0x80087467
> and to compare an int with a long, the sign is extended to 64bit,
> so the comparison is:
>
> 	if (0xffffffff80087467 == 0x0000000080087467)
>
> which doesn't match whereas it should.
>
> This patch uses int in the case of the target command type
> instead of abi_long (and for consistency, update IOCTLEntry).
>
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>

Applied to linux-user que,

Thanks

> ---
> v2: use int instead of abi_int, as it is recommended by Peter Maydell.
>
>  linux-user/syscall.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 1622ad6..c28cd05 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -3290,11 +3290,11 @@ enum {
>  typedef struct IOCTLEntry IOCTLEntry;
>  
>  typedef abi_long do_ioctl_fn(const IOCTLEntry *ie, uint8_t *buf_temp,
> -                             int fd, abi_long cmd, abi_long arg);
> +                             int fd, int cmd, abi_long arg);
>  
>  struct IOCTLEntry {
>      int target_cmd;
> -    unsigned int host_cmd;
> +    int host_cmd;
>      const char *name;
>      int access;
>      do_ioctl_fn *do_ioctl;
> @@ -3316,7 +3316,7 @@ struct IOCTLEntry {
>                              / sizeof(struct fiemap_extent))
>  
>  static abi_long do_ioctl_fs_ioc_fiemap(const IOCTLEntry *ie, 
> uint8_t *buf_temp,
> -                                       int fd, abi_long cmd, abi_long arg)
> +                                       int fd, int cmd, abi_long arg)
>  {
>      /* The parameter for this ioctl is a struct fiemap followed
>       * by an array of struct fiemap_extent whose size is set
> @@ -3397,7 +3397,7 @@ static abi_long 
> do_ioctl_fs_ioc_fiemap(const IOCTLEntry *ie, uint8_t *buf_temp,
>  #endif
>  
>  static abi_long do_ioctl_ifconf(const IOCTLEntry *ie, uint8_t *buf_temp,
> -                                int fd, abi_long cmd, abi_long arg)
> +                                int fd, int cmd, abi_long arg)
>  {
>      const argtype *arg_type = ie->arg_type;
>      int target_size;
> @@ -3491,7 +3491,7 @@ static abi_long do_ioctl_ifconf(const 
> IOCTLEntry *ie, uint8_t *buf_temp,
>  }
>  
>  static abi_long do_ioctl_dm(const IOCTLEntry *ie, uint8_t 
> *buf_temp, int fd,
> -                            abi_long cmd, abi_long arg)
> +                            int cmd, abi_long arg)
>  {
>      void *argptr;
>      struct dm_ioctl *host_dm;
> @@ -3716,7 +3716,7 @@ out:
>  }
>  
>  static abi_long do_ioctl_blkpg(const IOCTLEntry *ie, uint8_t 
> *buf_temp, int fd,
> -                               abi_long cmd, abi_long arg)
> +                               int cmd, abi_long arg)
>  {
>      void *argptr;
>      int target_size;
> @@ -3769,7 +3769,7 @@ out:
>  }
>  
>  static abi_long do_ioctl_rt(const IOCTLEntry *ie, uint8_t *buf_temp,
> -                                int fd, abi_long cmd, abi_long arg)
> +                                int fd, int cmd, abi_long arg)
>  {
>      const argtype *arg_type = ie->arg_type;
>      const StructEntry *se;
> @@ -3832,7 +3832,7 @@ static abi_long do_ioctl_rt(const 
> IOCTLEntry *ie, uint8_t *buf_temp,
>  }
>  
>  static abi_long do_ioctl_kdsigaccept(const IOCTLEntry *ie, 
> uint8_t *buf_temp,
> -                                     int fd, abi_long cmd, abi_long arg)
> +                                     int fd, int cmd, abi_long arg)
>  {
>      int sig = target_to_host_signal(arg);
>      return get_errno(ioctl(fd, ie->host_cmd, sig));
> @@ -3849,7 +3849,7 @@ static IOCTLEntry ioctl_entries[] = {
>  
>  /* ??? Implement proper locking for ioctls.  */
>  /* do_ioctl() Must return target values and target errnos. */
> -static abi_long do_ioctl(int fd, abi_long cmd, abi_long arg)
> +static abi_long do_ioctl(int fd, int cmd, abi_long arg)
>  {
>      const IOCTLEntry *ie;
>      const argtype *arg_type;