include/exec/cpu_ldst.h | 4 ++ linux-user/ioctls.h | 11 ++- linux-user/mips/signal.c | 5 +- linux-user/qemu.h | 4 +- linux-user/syscall.c | 140 +++++++++++++++++++++++++++++-------- linux-user/syscall_defs.h | 30 +++++++- linux-user/syscall_types.h | 6 -- 7 files changed, 158 insertions(+), 42 deletions(-)
The following changes since commit a1a4d49f60d2b899620ee2be4ebb991c4a90a026: Merge remote-tracking branch 'remotes/philmd-gitlab/tags/pflash-next-20190716' into staging (2019-07-16 17:02:44 +0100) are available in the Git repository at: git://github.com/vivier/qemu.git tags/linux-user-for-4.1-pull-request for you to fetch changes up to ad0bcf5d59f120d546be7a2c3590afc66eea0b01: linux-user: check valid address in access_ok() (2019-07-17 09:02:51 +0200) ---------------------------------------------------------------- fix access_ok() to allow to run LTP on AARCH64, fix SIOCGSTAMP with 5.2 kernel headers, fix structure target_ucontext for MIPS ---------------------------------------------------------------- Aleksandar Markovic (1): linux-user: Fix structure target_ucontext for MIPS Daniel P. Berrangé (1): linux-user: fix to handle variably sized SIOCGSTAMP with new kernels Rémi Denis-Courmont (1): linux-user: check valid address in access_ok() include/exec/cpu_ldst.h | 4 ++ linux-user/ioctls.h | 11 ++- linux-user/mips/signal.c | 5 +- linux-user/qemu.h | 4 +- linux-user/syscall.c | 140 +++++++++++++++++++++++++++++-------- linux-user/syscall_defs.h | 30 +++++++- linux-user/syscall_types.h | 6 -- 7 files changed, 158 insertions(+), 42 deletions(-) -- 2.21.0
On Wed, 17 Jul 2019 at 15:55, Laurent Vivier <laurent@vivier.eu> wrote: > > The following changes since commit a1a4d49f60d2b899620ee2be4ebb991c4a90a026: > > Merge remote-tracking branch 'remotes/philmd-gitlab/tags/pflash-next-20190716' into staging (2019-07-16 17:02:44 +0100) > > are available in the Git repository at: > > git://github.com/vivier/qemu.git tags/linux-user-for-4.1-pull-request > > for you to fetch changes up to ad0bcf5d59f120d546be7a2c3590afc66eea0b01: > > linux-user: check valid address in access_ok() (2019-07-17 09:02:51 +0200) > > ---------------------------------------------------------------- > fix access_ok() to allow to run LTP on AARCH64, > fix SIOCGSTAMP with 5.2 kernel headers, > fix structure target_ucontext for MIPS > > --------------------------------------------------------------- This causes 'make check-tcg' to produce new warnings from running the tests (x86-64 host): RUN tests for x86_64 TEST test-mmap (default) on x86_64 ERROR: ioctl(IOCGSTAMP_NEW): target=0x80108906 host=0x8906 ERROR: ioctl(IOCGSTAMPNS_NEW): target=0x80108907 host=0x8907 TEST sha1 on x86_64 ERROR: ioctl(IOCGSTAMP_NEW): target=0x80108906 host=0x8906 ERROR: ioctl(IOCGSTAMPNS_NEW): target=0x80108907 host=0x8907 TEST linux-test on x86_64 ERROR: ioctl(IOCGSTAMP_NEW): target=0x80108906 host=0x8906 ERROR: ioctl(IOCGSTAMPNS_NEW): target=0x80108907 host=0x8907 TEST testthread on x86_64 ERROR: ioctl(IOCGSTAMP_NEW): target=0x80108906 host=0x8906 ERROR: ioctl(IOCGSTAMPNS_NEW): target=0x80108907 host=0x8907 TEST test-x86_64 on x86_64 ERROR: ioctl(IOCGSTAMP_NEW): target=0x80108906 host=0x8906 ERROR: ioctl(IOCGSTAMPNS_NEW): target=0x80108907 host=0x8907 TEST test-mmap (4096 byte pages) on x86_64 ERROR: ioctl(IOCGSTAMP_NEW): target=0x80108906 host=0x8906 ERROR: ioctl(IOCGSTAMPNS_NEW): target=0x80108907 host=0x8907 thanks -- PMM
Le 18/07/2019 à 12:20, Peter Maydell a écrit :
> On Wed, 17 Jul 2019 at 15:55, Laurent Vivier <laurent@vivier.eu> wrote:
>>
>> The following changes since commit a1a4d49f60d2b899620ee2be4ebb991c4a90a026:
>>
>> Merge remote-tracking branch 'remotes/philmd-gitlab/tags/pflash-next-20190716' into staging (2019-07-16 17:02:44 +0100)
>>
>> are available in the Git repository at:
>>
>> git://github.com/vivier/qemu.git tags/linux-user-for-4.1-pull-request
>>
>> for you to fetch changes up to ad0bcf5d59f120d546be7a2c3590afc66eea0b01:
>>
>> linux-user: check valid address in access_ok() (2019-07-17 09:02:51 +0200)
>>
>> ----------------------------------------------------------------
>> fix access_ok() to allow to run LTP on AARCH64,
>> fix SIOCGSTAMP with 5.2 kernel headers,
>> fix structure target_ucontext for MIPS
>>
>> ---------------------------------------------------------------
>
> This causes 'make check-tcg' to produce new warnings from
> running the tests (x86-64 host):
>
> RUN tests for x86_64
> TEST test-mmap (default) on x86_64
> ERROR: ioctl(IOCGSTAMP_NEW): target=0x80108906 host=0x8906
> ERROR: ioctl(IOCGSTAMPNS_NEW): target=0x80108907 host=0x8907
> TEST sha1 on x86_64
> ERROR: ioctl(IOCGSTAMP_NEW): target=0x80108906 host=0x8906
> ERROR: ioctl(IOCGSTAMPNS_NEW): target=0x80108907 host=0x8907
> TEST linux-test on x86_64
> ERROR: ioctl(IOCGSTAMP_NEW): target=0x80108906 host=0x8906
> ERROR: ioctl(IOCGSTAMPNS_NEW): target=0x80108907 host=0x8907
> TEST testthread on x86_64
> ERROR: ioctl(IOCGSTAMP_NEW): target=0x80108906 host=0x8906
> ERROR: ioctl(IOCGSTAMPNS_NEW): target=0x80108907 host=0x8907
> TEST test-x86_64 on x86_64
> ERROR: ioctl(IOCGSTAMP_NEW): target=0x80108906 host=0x8906
> ERROR: ioctl(IOCGSTAMPNS_NEW): target=0x80108907 host=0x8907
> TEST test-mmap (4096 byte pages) on x86_64
> ERROR: ioctl(IOCGSTAMP_NEW): target=0x80108906 host=0x8906
> ERROR: ioctl(IOCGSTAMPNS_NEW): target=0x80108907 host=0x8907
It comes from linux-user/syscall.c:
6328 /* automatic consistency check if same arch */
6329 #if (defined(__i386__) && defined(TARGET_I386) && defined(TARGET_ABI32)) || \
6330 (defined(__x86_64__) && defined(TARGET_X86_64))
6331 if (unlikely(ie->target_cmd != ie->host_cmd)) {
6332 fprintf(stderr, "ERROR: ioctl(%s): target=0x%x host=0x%x\n",
6333 ie->name, ie->target_cmd, ie->host_cmd);
6334 }
6335 #endif
because of:
+ { TARGET_SIOCGSTAMP_OLD, SIOCGSTAMP, "IOCGSTAMP_OLD", IOC_R, \
+ do_ioctl_SIOCGSTAMP },
+ { TARGET_SIOCGSTAMPNS_OLD, SIOCGSTAMPNS, "IOCGSTAMPNS_OLD", IOC_R, \
+ do_ioctl_SIOCGSTAMPNS },
+ { TARGET_SIOCGSTAMP_NEW, SIOCGSTAMP, "IOCGSTAMP_NEW", IOC_R, \
+ do_ioctl_SIOCGSTAMP },
+ { TARGET_SIOCGSTAMPNS_NEW, SIOCGSTAMPNS, "IOCGSTAMPNS_NEW", IOC_R, \
+ do_ioctl_SIOCGSTAMPNS },
As the host_cmd is not used, the simplest way to fix that is
+ { TARGET_SIOCGSTAMP_OLD, TARGET_SIOCGSTAMP_OLD, "IOCGSTAMP_OLD", IOC_R, \
+ do_ioctl_SIOCGSTAMP },
+ { TARGET_SIOCGSTAMPNS_OLD, TARGET_SIOCGSTAMPNS_OLD, "IOCGSTAMPNS_OLD", IOC_R, \
+ do_ioctl_SIOCGSTAMPNS },
+ { TARGET_SIOCGSTAMP_NEW, TARGET_SIOCGSTAMP_NEW, "IOCGSTAMP_NEW", IOC_R, \
+ do_ioctl_SIOCGSTAMP },
+ { TARGET_SIOCGSTAMPNS_NEW, TARGET_SIOCGSTAMPNS_NEW, "IOCGSTAMPNS_NEW", IOC_R, \
+ do_ioctl_SIOCGSTAMPNS },
As SIOCGSTAMP_OLD and SIOCGSTAMP_NEW can be undefined on the host (and not needed
because we always use SIOCGSTAMP and SIOCGSTAMPNS)
Thanks,
Laurent
On Thu, 18 Jul 2019 at 11:40, Laurent Vivier <laurent@vivier.eu> wrote:
> It comes from linux-user/syscall.c:
>
> 6328 /* automatic consistency check if same arch */
> 6329 #if (defined(__i386__) && defined(TARGET_I386) && defined(TARGET_ABI32)) || \
> 6330 (defined(__x86_64__) && defined(TARGET_X86_64))
> 6331 if (unlikely(ie->target_cmd != ie->host_cmd)) {
> 6332 fprintf(stderr, "ERROR: ioctl(%s): target=0x%x host=0x%x\n",
> 6333 ie->name, ie->target_cmd, ie->host_cmd);
> 6334 }
> 6335 #endif
>
> because of:
>
> + { TARGET_SIOCGSTAMP_OLD, SIOCGSTAMP, "IOCGSTAMP_OLD", IOC_R, \
> + do_ioctl_SIOCGSTAMP },
> + { TARGET_SIOCGSTAMPNS_OLD, SIOCGSTAMPNS, "IOCGSTAMPNS_OLD", IOC_R, \
> + do_ioctl_SIOCGSTAMPNS },
> + { TARGET_SIOCGSTAMP_NEW, SIOCGSTAMP, "IOCGSTAMP_NEW", IOC_R, \
> + do_ioctl_SIOCGSTAMP },
> + { TARGET_SIOCGSTAMPNS_NEW, SIOCGSTAMPNS, "IOCGSTAMPNS_NEW", IOC_R, \
> + do_ioctl_SIOCGSTAMPNS },
>
> As the host_cmd is not used, the simplest way to fix that is
>
> + { TARGET_SIOCGSTAMP_OLD, TARGET_SIOCGSTAMP_OLD, "IOCGSTAMP_OLD", IOC_R, \
> + do_ioctl_SIOCGSTAMP },
> + { TARGET_SIOCGSTAMPNS_OLD, TARGET_SIOCGSTAMPNS_OLD, "IOCGSTAMPNS_OLD", IOC_R, \
> + do_ioctl_SIOCGSTAMPNS },
> + { TARGET_SIOCGSTAMP_NEW, TARGET_SIOCGSTAMP_NEW, "IOCGSTAMP_NEW", IOC_R, \
> + do_ioctl_SIOCGSTAMP },
> + { TARGET_SIOCGSTAMPNS_NEW, TARGET_SIOCGSTAMPNS_NEW, "IOCGSTAMPNS_NEW", IOC_R, \
> + do_ioctl_SIOCGSTAMPNS },
>
> As SIOCGSTAMP_OLD and SIOCGSTAMP_NEW can be undefined on the host (and not needed
> because we always use SIOCGSTAMP and SIOCGSTAMPNS)
So we don't use the host_cmd because we have a custom do_ioctl_foo
function which doesn't look at that field?
Sounds OK, but please include a comment explaining why.
PS: why didn't you use IOCTL_SPECIAL() rather than hand-written
array entries? None of the other ioctls.h entries do that.
Of course now we're trying to sidestep the consistency check
we can't use the macro, but it wolud have been fine otherwise.
It also would get the names of the ioctls in the string form
right -- they are all missing the initial "S" here.
Perhaps for 4.2 it might be worth considering having a
macro for "IOCTL_SPECIAL but skip the consistency check"
to be a bit less hacky here.
thanks
-- PMM
Le 18/07/2019 à 13:00, Peter Maydell a écrit :
> On Thu, 18 Jul 2019 at 11:40, Laurent Vivier <laurent@vivier.eu> wrote:
>> It comes from linux-user/syscall.c:
>>
>> 6328 /* automatic consistency check if same arch */
>> 6329 #if (defined(__i386__) && defined(TARGET_I386) && defined(TARGET_ABI32)) || \
>> 6330 (defined(__x86_64__) && defined(TARGET_X86_64))
>> 6331 if (unlikely(ie->target_cmd != ie->host_cmd)) {
>> 6332 fprintf(stderr, "ERROR: ioctl(%s): target=0x%x host=0x%x\n",
>> 6333 ie->name, ie->target_cmd, ie->host_cmd);
>> 6334 }
>> 6335 #endif
>>
>> because of:
>>
>> + { TARGET_SIOCGSTAMP_OLD, SIOCGSTAMP, "IOCGSTAMP_OLD", IOC_R, \
>> + do_ioctl_SIOCGSTAMP },
>> + { TARGET_SIOCGSTAMPNS_OLD, SIOCGSTAMPNS, "IOCGSTAMPNS_OLD", IOC_R, \
>> + do_ioctl_SIOCGSTAMPNS },
>> + { TARGET_SIOCGSTAMP_NEW, SIOCGSTAMP, "IOCGSTAMP_NEW", IOC_R, \
>> + do_ioctl_SIOCGSTAMP },
>> + { TARGET_SIOCGSTAMPNS_NEW, SIOCGSTAMPNS, "IOCGSTAMPNS_NEW", IOC_R, \
>> + do_ioctl_SIOCGSTAMPNS },
>>
>> As the host_cmd is not used, the simplest way to fix that is
>>
>> + { TARGET_SIOCGSTAMP_OLD, TARGET_SIOCGSTAMP_OLD, "IOCGSTAMP_OLD", IOC_R, \
>> + do_ioctl_SIOCGSTAMP },
>> + { TARGET_SIOCGSTAMPNS_OLD, TARGET_SIOCGSTAMPNS_OLD, "IOCGSTAMPNS_OLD", IOC_R, \
>> + do_ioctl_SIOCGSTAMPNS },
>> + { TARGET_SIOCGSTAMP_NEW, TARGET_SIOCGSTAMP_NEW, "IOCGSTAMP_NEW", IOC_R, \
>> + do_ioctl_SIOCGSTAMP },
>> + { TARGET_SIOCGSTAMPNS_NEW, TARGET_SIOCGSTAMPNS_NEW, "IOCGSTAMPNS_NEW", IOC_R, \
>> + do_ioctl_SIOCGSTAMPNS },
>>
>> As SIOCGSTAMP_OLD and SIOCGSTAMP_NEW can be undefined on the host (and not needed
>> because we always use SIOCGSTAMP and SIOCGSTAMPNS)
>
> So we don't use the host_cmd because we have a custom do_ioctl_foo
> function which doesn't look at that field?
Yes
> Sounds OK, but please include a comment explaining why.
OK
> PS: why didn't you use IOCTL_SPECIAL() rather than hand-written
> array entries? None of the other ioctls.h entries do that.
because IOCTL_SPECIAL() extracts the host command from the parameter and
so we would need the _NEW and _OLD definitions on the host, and they are
not available with kernel before 5.2
> Of course now we're trying to sidestep the consistency check
> we can't use the macro, but it wolud have been fine otherwise.
> It also would get the names of the ioctls in the string form
> right -- they are all missing the initial "S" here.
oh, yes. I fix that too.
> Perhaps for 4.2 it might be worth considering having a
> macro for "IOCTL_SPECIAL but skip the consistency check"
> to be a bit less hacky here.
Perhaps, rather than using TARGET_XXX in host_cmd I can use NULL and
check for NULL in consistency check?
Do you think we should defer the whole patch after 4.1 release?
But then the build of 4.1 will be broken with 5.2+ kernel
Thanks,
Laurent
On Thu, 18 Jul 2019 at 12:10, Laurent Vivier <laurent@vivier.eu> wrote: > Do you think we should defer the whole patch after 4.1 release? > But then the build of 4.1 will be broken with 5.2+ kernel I think this is worth putting into 4.1; but we should look at maybe tidying up the loose ends for 4.2. thanks -- PMM
© 2016 - 2024 Red Hat, Inc.