* [Qemu-devel] [PATCH v2] linux-user: ioctl() command type is int
2015-05-21 22:18 [Qemu-devel] [PATCH] " Laurent Vivier
@ 2015-05-23 13:17 ` Laurent Vivier
2015-05-23 21:07 ` Peter Maydell
2015-06-12 13:06 ` Riku Voipio
0 siblings, 2 replies; 15+ messages in thread
From: Laurent Vivier @ 2015-05-23 13:17 UTC (permalink / raw)
To: peter.maydell; +Cc: eswierk, riku.voipio, qemu-devel, Laurent Vivier
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(-)
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;
--
2.1.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH v2] linux-user: ioctl() command type is int
2015-05-23 13:17 ` [Qemu-devel] [PATCH v2] " Laurent Vivier
@ 2015-05-23 21:07 ` Peter Maydell
2015-06-12 13:06 ` Riku Voipio
1 sibling, 0 replies; 15+ messages in thread
From: Peter Maydell @ 2015-05-23 21:07 UTC (permalink / raw)
To: Laurent Vivier; +Cc: eswierk, Riku Voipio, QEMU Developers
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH v2] linux-user: ioctl() command type is int
2015-05-23 13:17 ` [Qemu-devel] [PATCH v2] " Laurent Vivier
2015-05-23 21:07 ` Peter Maydell
@ 2015-06-12 13:06 ` Riku Voipio
1 sibling, 0 replies; 15+ messages in thread
From: Riku Voipio @ 2015-06-12 13:06 UTC (permalink / raw)
To: Laurent Vivier; +Cc: peter.maydell, eswierk, qemu-devel
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;
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PULL 0/6] linux-user patches for 2.4 softfreeze
@ 2015-06-15 12:20 riku.voipio
2015-06-15 12:20 ` [Qemu-devel] [PULL 1/6] linux-user: Allocate thunk size dynamically riku.voipio
` (6 more replies)
0 siblings, 7 replies; 15+ messages in thread
From: riku.voipio @ 2015-06-15 12:20 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Riku Voipio
From: Riku Voipio <riku.voipio@linaro.org>
The following changes since commit 0a2df857a7038c75379cc575de5d4be4c0ac629e:
Merge remote-tracking branch 'remotes/stefanha/tags/net-pull-request' into staging (2015-06-12 15:39:05 +0100)
are available in the git repository at:
git://git.linaro.org/people/riku.voipio/qemu.git tags/pull-linux-user-20150615
for you to fetch changes up to d2897da1f1e97d684f80ff62d473c31b79bc643a:
linux-user: fix the breakpoint inheritance in spawned threads (2015-06-15 11:36:59 +0300)
----------------------------------------------------------------
linux-user patches for 2.4 softfreeze
----------------------------------------------------------------
Alexander Graf (1):
linux-user: Allocate thunk size dynamically
Laurent Vivier (1):
linux-user: ioctl() command type is int
Peter Maydell (2):
linux-user: Fix length handling in host_to_target_cmsg
linux-user: use __get_user and __put_user in cmsg conversions
Thierry Bultel (1):
linux-user: fix the breakpoint inheritance in spawned threads
Yongbok Kim (1):
linux-user: Use abi_ulong for TARGET_ELF_PAGESTART
include/exec/user/thunk.h | 4 +-
linux-user/elfload.c | 3 +-
linux-user/main.c | 4 +-
linux-user/syscall.c | 110 +++++++++++++++++++++++++++++++++++-----------
thunk.c | 16 +++++--
5 files changed, 103 insertions(+), 34 deletions(-)
--
2.1.4
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PULL 1/6] linux-user: Allocate thunk size dynamically
2015-06-15 12:20 [Qemu-devel] [PULL 0/6] linux-user patches for 2.4 softfreeze riku.voipio
@ 2015-06-15 12:20 ` riku.voipio
2015-06-15 12:20 ` [Qemu-devel] [PULL 2/6] linux-user: Use abi_ulong for TARGET_ELF_PAGESTART riku.voipio
` (5 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: riku.voipio @ 2015-06-15 12:20 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Alexander Graf
From: Alexander Graf <agraf@suse.de>
We store all struct types in an array of static size without ever
checking whether we overrun it. Of course some day someone (like me
in another, ancient ALSA enabling patch set) will run into the limit
without realizing it.
So let's make the allocation dynamic. We already know the number of
structs that we want to allocate, so we only need to pass the variable
into the respective piece of code.
Also, to ensure we don't accidently overwrite random memory, add some
asserts to sanity check whether a thunk is actually part of our array.
Signed-off-by: Alexander Graf <agraf@suse.de>
Signed-off-by: Riku Voipio <riku.voipio@linaro.org>
---
include/exec/user/thunk.h | 4 +++-
linux-user/syscall.c | 3 +++
thunk.c | 16 ++++++++++++----
3 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/include/exec/user/thunk.h b/include/exec/user/thunk.h
index 87025c3..3b67462 100644
--- a/include/exec/user/thunk.h
+++ b/include/exec/user/thunk.h
@@ -74,7 +74,7 @@ const argtype *thunk_convert(void *dst, const void *src,
const argtype *type_ptr, int to_host);
#ifndef NO_THUNK_TYPE_SIZE
-extern StructEntry struct_entries[];
+extern StructEntry *struct_entries;
int thunk_type_size_array(const argtype *type_ptr, int is_host);
int thunk_type_align_array(const argtype *type_ptr, int is_host);
@@ -186,4 +186,6 @@ unsigned int target_to_host_bitmask(unsigned int x86_mask,
unsigned int host_to_target_bitmask(unsigned int alpha_mask,
const bitmask_transtbl * trans_tbl);
+void thunk_init(unsigned int max_structs);
+
#endif
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 1622ad6..f56f3e0 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -3277,6 +3277,7 @@ static abi_long do_ipc(unsigned int call, abi_long first,
#define STRUCT_SPECIAL(name) STRUCT_ ## name,
enum {
#include "syscall_types.h"
+STRUCT_MAX
};
#undef STRUCT
#undef STRUCT_SPECIAL
@@ -4879,6 +4880,8 @@ void syscall_init(void)
int size;
int i;
+ thunk_init(STRUCT_MAX);
+
#define STRUCT(name, ...) thunk_register_struct(STRUCT_ ## name, #name, struct_ ## name ## _def);
#define STRUCT_SPECIAL(name) thunk_register_struct_direct(STRUCT_ ## name, #name, &struct_ ## name ## _def);
#include "syscall_types.h"
diff --git a/thunk.c b/thunk.c
index 3cca047..f501fd7 100644
--- a/thunk.c
+++ b/thunk.c
@@ -25,10 +25,8 @@
//#define DEBUG
-#define MAX_STRUCTS 128
-
-/* XXX: make it dynamic */
-StructEntry struct_entries[MAX_STRUCTS];
+static unsigned int max_struct_entries;
+StructEntry *struct_entries;
static const argtype *thunk_type_next_ptr(const argtype *type_ptr);
@@ -70,6 +68,7 @@ void thunk_register_struct(int id, const char *name, const argtype *types)
StructEntry *se;
int nb_fields, offset, max_align, align, size, i, j;
+ assert(id < max_struct_entries);
se = struct_entries + id;
/* first we count the number of fields */
@@ -117,6 +116,8 @@ void thunk_register_struct_direct(int id, const char *name,
const StructEntry *se1)
{
StructEntry *se;
+
+ assert(id < max_struct_entries);
se = struct_entries + id;
*se = *se1;
se->name = name;
@@ -244,6 +245,7 @@ const argtype *thunk_convert(void *dst, const void *src,
const argtype *field_types;
const int *dst_offsets, *src_offsets;
+ assert(*type_ptr < max_struct_entries);
se = struct_entries + *type_ptr++;
if (se->convert[0] != NULL) {
/* specific conversion is needed */
@@ -314,3 +316,9 @@ int thunk_type_align_array(const argtype *type_ptr, int is_host)
return thunk_type_align(type_ptr, is_host);
}
#endif /* ndef NO_THUNK_TYPE_SIZE */
+
+void thunk_init(unsigned int max_structs)
+{
+ max_struct_entries = max_structs;
+ struct_entries = g_new0(StructEntry, max_structs);
+}
--
2.1.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PULL 2/6] linux-user: Use abi_ulong for TARGET_ELF_PAGESTART
2015-06-15 12:20 [Qemu-devel] [PULL 0/6] linux-user patches for 2.4 softfreeze riku.voipio
2015-06-15 12:20 ` [Qemu-devel] [PULL 1/6] linux-user: Allocate thunk size dynamically riku.voipio
@ 2015-06-15 12:20 ` riku.voipio
2015-06-15 12:20 ` [Qemu-devel] [PULL 3/6] linux-user: ioctl() command type is int riku.voipio
` (4 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: riku.voipio @ 2015-06-15 12:20 UTC (permalink / raw)
To: qemu-devel; +Cc: Yongbok Kim, peter.maydell
From: Yongbok Kim <yongbok.kim@imgtec.com>
TARGET_ELF_PAGESTART is required to use abi_ulong to correctly handle
addresses for different target bits width.
This patch fixes a problem when running a 64-bit user mode application
on 32-bit host machines.
Signed-off-by: Yongbok Kim <yongbok.kim@imgtec.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Riku Voipio <riku.voipio@linaro.org>
---
linux-user/elfload.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index b71e866..1788368 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -1256,7 +1256,8 @@ struct exec
/* Necessary parameters */
#define TARGET_ELF_EXEC_PAGESIZE TARGET_PAGE_SIZE
-#define TARGET_ELF_PAGESTART(_v) ((_v) & ~(unsigned long)(TARGET_ELF_EXEC_PAGESIZE-1))
+#define TARGET_ELF_PAGESTART(_v) ((_v) & \
+ ~(abi_ulong)(TARGET_ELF_EXEC_PAGESIZE-1))
#define TARGET_ELF_PAGEOFFSET(_v) ((_v) & (TARGET_ELF_EXEC_PAGESIZE-1))
#define DLINFO_ITEMS 14
--
2.1.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PULL 3/6] linux-user: ioctl() command type is int
2015-06-15 12:20 [Qemu-devel] [PULL 0/6] linux-user patches for 2.4 softfreeze riku.voipio
2015-06-15 12:20 ` [Qemu-devel] [PULL 1/6] linux-user: Allocate thunk size dynamically riku.voipio
2015-06-15 12:20 ` [Qemu-devel] [PULL 2/6] linux-user: Use abi_ulong for TARGET_ELF_PAGESTART riku.voipio
@ 2015-06-15 12:20 ` riku.voipio
2015-06-15 12:20 ` [Qemu-devel] [PULL 4/6] linux-user: Fix length handling in host_to_target_cmsg riku.voipio
` (3 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: riku.voipio @ 2015-06-15 12:20 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Laurent Vivier
From: Laurent Vivier <laurent@vivier.eu>
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>
Signed-off-by: Riku Voipio <riku.voipio@linaro.org>
---
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 f56f3e0..ed8c423 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -3291,11 +3291,11 @@ STRUCT_MAX
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;
@@ -3317,7 +3317,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
@@ -3398,7 +3398,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;
@@ -3492,7 +3492,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;
@@ -3717,7 +3717,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;
@@ -3770,7 +3770,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;
@@ -3833,7 +3833,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));
@@ -3850,7 +3850,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;
--
2.1.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PULL 4/6] linux-user: Fix length handling in host_to_target_cmsg
2015-06-15 12:20 [Qemu-devel] [PULL 0/6] linux-user patches for 2.4 softfreeze riku.voipio
` (2 preceding siblings ...)
2015-06-15 12:20 ` [Qemu-devel] [PULL 3/6] linux-user: ioctl() command type is int riku.voipio
@ 2015-06-15 12:20 ` riku.voipio
2015-06-15 12:20 ` [Qemu-devel] [PULL 5/6] linux-user: use __get_user and __put_user in cmsg conversions riku.voipio
` (2 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: riku.voipio @ 2015-06-15 12:20 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell
From: Peter Maydell <peter.maydell@linaro.org>
The previous code for handling payload length when converting
cmsg structures from host to target had a number of problems:
* we required the msg->msg_controllen to declare the buffer
to have enough space for final trailing padding (we were
checking against CMSG_SPACE), whereas the kernel does not
require this, and common userspace code assumes this. (In
particular, glibc's "try to talk to nscd" code that it will
run on startup will receive a cmsg with a 4 byte payload and
only allocate 4 bytes for it, which was causing us to do
the wrong thing on architectures that need 8-alignment.)
* we weren't correctly handling the fact that the SO_TIMESTAMP
payload may be larger for the target than the host
* we weren't marking the messages with MSG_CTRUNC when we did
need to truncate a message that wasn't truncated by the host,
but were instead logging a QEMU message; since truncation is
always the result of a guest giving us an insufficiently
sized buffer, we should report it to the guest as the kernel
does and don't log anything
Rewrite the parts of the function that deal with length to
fix these issues, and add a comment in target_to_host_cmsg
to explain why the overflow logging it does is a QEMU bug,
not a guest issue.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Riku Voipio <riku.voipio@linaro.org>
---
linux-user/syscall.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 61 insertions(+), 8 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index ed8c423..839ce93 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1202,6 +1202,15 @@ static inline abi_long target_to_host_cmsg(struct msghdr *msgh,
space += CMSG_SPACE(len);
if (space > msgh->msg_controllen) {
space -= CMSG_SPACE(len);
+ /* This is a QEMU bug, since we allocated the payload
+ * area ourselves (unlike overflow in host-to-target
+ * conversion, which is just the guest giving us a buffer
+ * that's too small). It can't happen for the payload types
+ * we currently support; if it becomes an issue in future
+ * we would need to improve our allocation strategy to
+ * something more intelligent than "twice the size of the
+ * target buffer we're reading from".
+ */
gemu_log("Host cmsg overflow\n");
break;
}
@@ -1267,11 +1276,16 @@ static inline abi_long host_to_target_cmsg(struct target_msghdr *target_msgh,
void *target_data = TARGET_CMSG_DATA(target_cmsg);
int len = cmsg->cmsg_len - CMSG_ALIGN(sizeof (struct cmsghdr));
+ int tgt_len, tgt_space;
- space += TARGET_CMSG_SPACE(len);
- if (space > msg_controllen) {
- space -= TARGET_CMSG_SPACE(len);
- gemu_log("Target cmsg overflow\n");
+ /* We never copy a half-header but may copy half-data;
+ * this is Linux's behaviour in put_cmsg(). Note that
+ * truncation here is a guest problem (which we report
+ * to the guest via the CTRUNC bit), unlike truncation
+ * in target_to_host_cmsg, which is a QEMU bug.
+ */
+ if (msg_controllen < sizeof(struct cmsghdr)) {
+ target_msgh->msg_flags |= tswap32(MSG_CTRUNC);
break;
}
@@ -1281,8 +1295,35 @@ static inline abi_long host_to_target_cmsg(struct target_msghdr *target_msgh,
target_cmsg->cmsg_level = tswap32(cmsg->cmsg_level);
}
target_cmsg->cmsg_type = tswap32(cmsg->cmsg_type);
- target_cmsg->cmsg_len = tswapal(TARGET_CMSG_LEN(len));
+ tgt_len = TARGET_CMSG_LEN(len);
+
+ /* Payload types which need a different size of payload on
+ * the target must adjust tgt_len here.
+ */
+ switch (cmsg->cmsg_level) {
+ case SOL_SOCKET:
+ switch (cmsg->cmsg_type) {
+ case SO_TIMESTAMP:
+ tgt_len = sizeof(struct target_timeval);
+ break;
+ default:
+ break;
+ }
+ default:
+ break;
+ }
+
+ if (msg_controllen < tgt_len) {
+ target_msgh->msg_flags |= tswap32(MSG_CTRUNC);
+ tgt_len = msg_controllen;
+ }
+
+ /* We must now copy-and-convert len bytes of payload
+ * into tgt_len bytes of destination space. Bear in mind
+ * that in both source and destination we may be dealing
+ * with a truncated value!
+ */
switch (cmsg->cmsg_level) {
case SOL_SOCKET:
switch (cmsg->cmsg_type) {
@@ -1290,7 +1331,7 @@ static inline abi_long host_to_target_cmsg(struct target_msghdr *target_msgh,
{
int *fd = (int *)data;
int *target_fd = (int *)target_data;
- int i, numfds = len / sizeof(int);
+ int i, numfds = tgt_len / sizeof(int);
for (i = 0; i < numfds; i++)
target_fd[i] = tswap32(fd[i]);
@@ -1302,8 +1343,10 @@ static inline abi_long host_to_target_cmsg(struct target_msghdr *target_msgh,
struct target_timeval *target_tv =
(struct target_timeval *)target_data;
- if (len != sizeof(struct timeval))
+ if (len != sizeof(struct timeval) ||
+ tgt_len != sizeof(struct target_timeval)) {
goto unimplemented;
+ }
/* copy struct timeval to target */
target_tv->tv_sec = tswapal(tv->tv_sec);
@@ -1330,9 +1373,19 @@ static inline abi_long host_to_target_cmsg(struct target_msghdr *target_msgh,
unimplemented:
gemu_log("Unsupported ancillary data: %d/%d\n",
cmsg->cmsg_level, cmsg->cmsg_type);
- memcpy(target_data, data, len);
+ memcpy(target_data, data, MIN(len, tgt_len));
+ if (tgt_len > len) {
+ memset(target_data + len, 0, tgt_len - len);
+ }
}
+ target_cmsg->cmsg_len = tswapal(tgt_len);
+ tgt_space = TARGET_CMSG_SPACE(tgt_len);
+ if (msg_controllen < tgt_space) {
+ tgt_space = msg_controllen;
+ }
+ msg_controllen -= tgt_space;
+ space += tgt_space;
cmsg = CMSG_NXTHDR(msgh, cmsg);
target_cmsg = TARGET_CMSG_NXTHDR(target_msgh, target_cmsg);
}
--
2.1.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PULL 5/6] linux-user: use __get_user and __put_user in cmsg conversions
2015-06-15 12:20 [Qemu-devel] [PULL 0/6] linux-user patches for 2.4 softfreeze riku.voipio
` (3 preceding siblings ...)
2015-06-15 12:20 ` [Qemu-devel] [PULL 4/6] linux-user: Fix length handling in host_to_target_cmsg riku.voipio
@ 2015-06-15 12:20 ` riku.voipio
2015-06-15 12:20 ` [Qemu-devel] [PULL 6/6] linux-user: fix the breakpoint inheritance in spawned threads riku.voipio
2015-06-15 15:14 ` [Qemu-devel] [PULL 0/6] linux-user patches for 2.4 softfreeze Peter Maydell
6 siblings, 0 replies; 15+ messages in thread
From: riku.voipio @ 2015-06-15 12:20 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell
From: Peter Maydell <peter.maydell@linaro.org>
The target payloads in cmsg conversions may not have the alignment
required by the host. Using the get_user and put_user functions is
the easiest way to handle this and also do the byte-swapping we
require.
(Note that prior to this commit target_to_host_cmsg was incorrectly
using __put_user() rather than __get_user() for the SCM_CREDENTIALS
conversion, which meant it wasn't getting the benefit of the
misalignment handling.)
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Riku Voipio <riku.voipio@linaro.org>
---
linux-user/syscall.c | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 839ce93..ed99104 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1228,17 +1228,18 @@ static inline abi_long target_to_host_cmsg(struct msghdr *msgh,
int *target_fd = (int *)target_data;
int i, numfds = len / sizeof(int);
- for (i = 0; i < numfds; i++)
- fd[i] = tswap32(target_fd[i]);
+ for (i = 0; i < numfds; i++) {
+ __get_user(fd[i], target_fd + i);
+ }
} else if (cmsg->cmsg_level == SOL_SOCKET
&& cmsg->cmsg_type == SCM_CREDENTIALS) {
struct ucred *cred = (struct ucred *)data;
struct target_ucred *target_cred =
(struct target_ucred *)target_data;
- __put_user(target_cred->pid, &cred->pid);
- __put_user(target_cred->uid, &cred->uid);
- __put_user(target_cred->gid, &cred->gid);
+ __get_user(cred->pid, &target_cred->pid);
+ __get_user(cred->uid, &target_cred->uid);
+ __get_user(cred->gid, &target_cred->gid);
} else {
gemu_log("Unsupported ancillary data: %d/%d\n",
cmsg->cmsg_level, cmsg->cmsg_type);
@@ -1333,8 +1334,9 @@ static inline abi_long host_to_target_cmsg(struct target_msghdr *target_msgh,
int *target_fd = (int *)target_data;
int i, numfds = tgt_len / sizeof(int);
- for (i = 0; i < numfds; i++)
- target_fd[i] = tswap32(fd[i]);
+ for (i = 0; i < numfds; i++) {
+ __put_user(fd[i], target_fd + i);
+ }
break;
}
case SO_TIMESTAMP:
@@ -1349,8 +1351,8 @@ static inline abi_long host_to_target_cmsg(struct target_msghdr *target_msgh,
}
/* copy struct timeval to target */
- target_tv->tv_sec = tswapal(tv->tv_sec);
- target_tv->tv_usec = tswapal(tv->tv_usec);
+ __put_user(tv->tv_sec, &target_tv->tv_sec);
+ __put_user(tv->tv_usec, &target_tv->tv_usec);
break;
}
case SCM_CREDENTIALS:
--
2.1.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PULL 6/6] linux-user: fix the breakpoint inheritance in spawned threads
2015-06-15 12:20 [Qemu-devel] [PULL 0/6] linux-user patches for 2.4 softfreeze riku.voipio
` (4 preceding siblings ...)
2015-06-15 12:20 ` [Qemu-devel] [PULL 5/6] linux-user: use __get_user and __put_user in cmsg conversions riku.voipio
@ 2015-06-15 12:20 ` riku.voipio
2015-06-15 15:14 ` [Qemu-devel] [PULL 0/6] linux-user patches for 2.4 softfreeze Peter Maydell
6 siblings, 0 replies; 15+ messages in thread
From: riku.voipio @ 2015-06-15 12:20 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Thierry Bultel
From: Thierry Bultel <thierry.bultel@basystemes.fr>
When a thread is spawned, cpu_copy re-initializes
the bp & wp lists of current thread, instead of the ones
of the new thread.
The effect is that breakpoints are no longer hit.
Signed-off-by: Thierry Bultel <thierry.bultel@basystemes.fr>
Signed-off-by: Riku Voipio <riku.voipio@linaro.org>
---
linux-user/main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/linux-user/main.c b/linux-user/main.c
index a0d3e58..c855bcc 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -3459,8 +3459,8 @@ CPUArchState *cpu_copy(CPUArchState *env)
/* Clone all break/watchpoints.
Note: Once we support ptrace with hw-debug register access, make sure
BP_CPU break/watchpoints are handled correctly on clone. */
- QTAILQ_INIT(&cpu->breakpoints);
- QTAILQ_INIT(&cpu->watchpoints);
+ QTAILQ_INIT(&new_cpu->breakpoints);
+ QTAILQ_INIT(&new_cpu->watchpoints);
QTAILQ_FOREACH(bp, &cpu->breakpoints, entry) {
cpu_breakpoint_insert(new_cpu, bp->pc, bp->flags, NULL);
}
--
2.1.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PULL 0/6] linux-user patches for 2.4 softfreeze
2015-06-15 12:20 [Qemu-devel] [PULL 0/6] linux-user patches for 2.4 softfreeze riku.voipio
` (5 preceding siblings ...)
2015-06-15 12:20 ` [Qemu-devel] [PULL 6/6] linux-user: fix the breakpoint inheritance in spawned threads riku.voipio
@ 2015-06-15 15:14 ` Peter Maydell
2015-06-15 15:26 ` Laurent Vivier
2015-06-15 22:35 ` [Qemu-devel] [PATCH v2] linux-user: ioctl() command type is int Laurent Vivier
6 siblings, 2 replies; 15+ messages in thread
From: Peter Maydell @ 2015-06-15 15:14 UTC (permalink / raw)
To: Riku Voipio; +Cc: QEMU Developers, Laurent Vivier
On 15 June 2015 at 13:20, <riku.voipio@linaro.org> wrote:
> From: Riku Voipio <riku.voipio@linaro.org>
>
> The following changes since commit 0a2df857a7038c75379cc575de5d4be4c0ac629e:
>
> Merge remote-tracking branch 'remotes/stefanha/tags/net-pull-request' into staging (2015-06-12 15:39:05 +0100)
>
> are available in the git repository at:
>
> git://git.linaro.org/people/riku.voipio/qemu.git tags/pull-linux-user-20150615
>
> for you to fetch changes up to d2897da1f1e97d684f80ff62d473c31b79bc643a:
>
> linux-user: fix the breakpoint inheritance in spawned threads (2015-06-15 11:36:59 +0300)
>
> ----------------------------------------------------------------
> linux-user patches for 2.4 softfreeze
>
> ----------------------------------------------------------------
I get a lot of build errors with clang:
/home/petmay01/linaro/qemu-for-merges/linux-user/syscall.c:3610:10:
error: overflow
converting case value to switch condition type (3241737481 to
18446744072656321801)
[-Werror,-Wswitch]
case DM_TABLE_LOAD:
^
/usr/include/linux/dm-ioctl.h:259:26: note: expanded from macro 'DM_TABLE_LOAD'
#define DM_TABLE_LOAD _IOWR(DM_IOCTL, DM_TABLE_LOAD_CMD, struct dm_ioctl)
^
/usr/include/asm-generic/ioctl.h:77:29: note: expanded from macro '_IOWR'
#define _IOWR(type,nr,size)
_IOC(_IOC_READ|_IOC_WRITE,(type),(nr),(_IOC_TYPECHE...
^
/usr/include/asm-generic/ioctl.h:66:2: note: expanded from macro '_IOC'
(((dir) << _IOC_DIRSHIFT) | \
^
/home/petmay01/linaro/qemu-for-merges/linux-user/syscall.c:3606:10:
error: overflow
converting case value to switch condition type (3241737486 to
18446744072656321806)
[-Werror,-Wswitch]
case DM_TARGET_MSG:
^
(etc etc for all the cases until clang gives up because it's emitted
too many errors).
Guessing this is the result of the ioctl patch?
thanks
-- PMM
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PULL 0/6] linux-user patches for 2.4 softfreeze
2015-06-15 15:14 ` [Qemu-devel] [PULL 0/6] linux-user patches for 2.4 softfreeze Peter Maydell
@ 2015-06-15 15:26 ` Laurent Vivier
2015-06-15 22:35 ` [Qemu-devel] [PATCH v2] linux-user: ioctl() command type is int Laurent Vivier
1 sibling, 0 replies; 15+ messages in thread
From: Laurent Vivier @ 2015-06-15 15:26 UTC (permalink / raw)
To: Peter Maydell, Riku Voipio; +Cc: QEMU Developers
Le 15/06/2015 17:14, Peter Maydell a écrit :
> On 15 June 2015 at 13:20, <riku.voipio@linaro.org> wrote:
>> From: Riku Voipio <riku.voipio@linaro.org>
>>
>> The following changes since commit 0a2df857a7038c75379cc575de5d4be4c0ac629e:
>>
>> Merge remote-tracking branch 'remotes/stefanha/tags/net-pull-request' into staging (2015-06-12 15:39:05 +0100)
>>
>> are available in the git repository at:
>>
>> git://git.linaro.org/people/riku.voipio/qemu.git tags/pull-linux-user-20150615
>>
>> for you to fetch changes up to d2897da1f1e97d684f80ff62d473c31b79bc643a:
>>
>> linux-user: fix the breakpoint inheritance in spawned threads (2015-06-15 11:36:59 +0300)
>>
>> ----------------------------------------------------------------
>> linux-user patches for 2.4 softfreeze
>>
>> ----------------------------------------------------------------
>
> I get a lot of build errors with clang:
>
> /home/petmay01/linaro/qemu-for-merges/linux-user/syscall.c:3610:10:
> error: overflow
> converting case value to switch condition type (3241737481 to
> 18446744072656321801)
> [-Werror,-Wswitch]
> case DM_TABLE_LOAD:
> ^
> /usr/include/linux/dm-ioctl.h:259:26: note: expanded from macro 'DM_TABLE_LOAD'
> #define DM_TABLE_LOAD _IOWR(DM_IOCTL, DM_TABLE_LOAD_CMD, struct dm_ioctl)
> ^
> /usr/include/asm-generic/ioctl.h:77:29: note: expanded from macro '_IOWR'
> #define _IOWR(type,nr,size)
> _IOC(_IOC_READ|_IOC_WRITE,(type),(nr),(_IOC_TYPECHE...
> ^
> /usr/include/asm-generic/ioctl.h:66:2: note: expanded from macro '_IOC'
> (((dir) << _IOC_DIRSHIFT) | \
> ^
> /home/petmay01/linaro/qemu-for-merges/linux-user/syscall.c:3606:10:
> error: overflow
> converting case value to switch condition type (3241737486 to
> 18446744072656321806)
> [-Werror,-Wswitch]
> case DM_TARGET_MSG:
> ^
> (etc etc for all the cases until clang gives up because it's emitted
> too many errors).
>
> Guessing this is the result of the ioctl patch?
Yes, I guess too.
Could drop this patch out ? I will rework it.
Laurent
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH v2] linux-user: ioctl() command type is int
2015-06-15 15:14 ` [Qemu-devel] [PULL 0/6] linux-user patches for 2.4 softfreeze Peter Maydell
2015-06-15 15:26 ` Laurent Vivier
@ 2015-06-15 22:35 ` Laurent Vivier
2015-06-15 22:46 ` Eric Blake
2015-06-16 6:57 ` Riku Voipio
1 sibling, 2 replies; 15+ messages in thread
From: Laurent Vivier @ 2015-06-15 22:35 UTC (permalink / raw)
To: peter.maydell, riku.voipio; +Cc: qemu-devel, Laurent Vivier
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.
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
v2: Don't modify IOCTLEntry type (useless and introduce clang errors)
linux-user/syscall.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index b98b7e7..5a280c3 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -3645,7 +3645,7 @@ 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;
@@ -3671,7 +3671,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
@@ -3752,7 +3752,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;
@@ -3846,7 +3846,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;
@@ -4071,7 +4071,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;
@@ -4124,7 +4124,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;
@@ -4187,7 +4187,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));
@@ -4204,7 +4204,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;
--
2.4.3
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH v2] linux-user: ioctl() command type is int
2015-06-15 22:35 ` [Qemu-devel] [PATCH v2] linux-user: ioctl() command type is int Laurent Vivier
@ 2015-06-15 22:46 ` Eric Blake
2015-06-16 6:57 ` Riku Voipio
1 sibling, 0 replies; 15+ messages in thread
From: Eric Blake @ 2015-06-15 22:46 UTC (permalink / raw)
To: Laurent Vivier, peter.maydell, riku.voipio; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1295 bytes --]
On 06/15/2015 04:35 PM, Laurent Vivier wrote:
> When executing a 64bit target chroot on 64bit host,
> the ioctl() command can mismatch.
>
>
> The origin of the problem is in syscall.c:do_ioctl().
>
> static abi_long do_ioctl(int fd, abi_long cmd, abi_long arg)
It's annoying that Linux picked ioctl(int, unsigned long request, ...),
particularly since POSIX picked ioctl(int, int request, ...) [1] and
therefore Linux is constrained to never accept a 'request' that doesn't
fit in 32 bits. Especially so since the POSIX definition of ioctl()
applies only to the obsolete STREAMS interface that Linux never really
picked up on. (The gnulib project has determined ways to write an
ioctl() wrapper that always takes an int request, then widens to long as
necessary before calling the real syscall, with no ill effects [2])
[1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/ioctl.html
[2] http://git.savannah.gnu.org/cgit/gnulib.git/tree/lib/ioctl.c
However, I don't feel comfortable enough with this code to give a
competent review, only to offer up that bit of trivia and the vague
impression that it looks like you are safe in this patch.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH v2] linux-user: ioctl() command type is int
2015-06-15 22:35 ` [Qemu-devel] [PATCH v2] linux-user: ioctl() command type is int Laurent Vivier
2015-06-15 22:46 ` Eric Blake
@ 2015-06-16 6:57 ` Riku Voipio
1 sibling, 0 replies; 15+ messages in thread
From: Riku Voipio @ 2015-06-16 6:57 UTC (permalink / raw)
To: Laurent Vivier; +Cc: Peter Maydell, qemu-devel qemu-devel
On 16 June 2015 at 01:35, 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.
>
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
> v2: Don't modify IOCTLEntry type (useless and introduce clang errors)
Thanks, tested to build with clang, will refresh pull request in a moment.
> linux-user/syscall.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index b98b7e7..5a280c3 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -3645,7 +3645,7 @@ 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;
> @@ -3671,7 +3671,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
> @@ -3752,7 +3752,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;
> @@ -3846,7 +3846,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;
> @@ -4071,7 +4071,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;
> @@ -4124,7 +4124,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;
> @@ -4187,7 +4187,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));
> @@ -4204,7 +4204,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;
> --
> 2.4.3
>
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2015-06-16 6:57 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-06-15 12:20 [Qemu-devel] [PULL 0/6] linux-user patches for 2.4 softfreeze riku.voipio
2015-06-15 12:20 ` [Qemu-devel] [PULL 1/6] linux-user: Allocate thunk size dynamically riku.voipio
2015-06-15 12:20 ` [Qemu-devel] [PULL 2/6] linux-user: Use abi_ulong for TARGET_ELF_PAGESTART riku.voipio
2015-06-15 12:20 ` [Qemu-devel] [PULL 3/6] linux-user: ioctl() command type is int riku.voipio
2015-06-15 12:20 ` [Qemu-devel] [PULL 4/6] linux-user: Fix length handling in host_to_target_cmsg riku.voipio
2015-06-15 12:20 ` [Qemu-devel] [PULL 5/6] linux-user: use __get_user and __put_user in cmsg conversions riku.voipio
2015-06-15 12:20 ` [Qemu-devel] [PULL 6/6] linux-user: fix the breakpoint inheritance in spawned threads riku.voipio
2015-06-15 15:14 ` [Qemu-devel] [PULL 0/6] linux-user patches for 2.4 softfreeze Peter Maydell
2015-06-15 15:26 ` Laurent Vivier
2015-06-15 22:35 ` [Qemu-devel] [PATCH v2] linux-user: ioctl() command type is int Laurent Vivier
2015-06-15 22:46 ` Eric Blake
2015-06-16 6:57 ` Riku Voipio
-- strict thread matches above, loose matches on Subject: below --
2015-05-21 22:18 [Qemu-devel] [PATCH] " Laurent Vivier
2015-05-23 13:17 ` [Qemu-devel] [PATCH v2] " Laurent Vivier
2015-05-23 21:07 ` Peter Maydell
2015-06-12 13:06 ` Riku Voipio
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).