Le 09/07/2020 à 22:00, Laurent Vivier a écrit :
> In new functions print_ioctl() and print_syscall_ret_ioctl(), we don't
> check if lock_user() returns NULL and this would cause a segfault in
> thunk_print().
>
> If lock_user() returns NULL don't call thunk_print() but prints only the
> value of the (invalid) pointer.
>
> Tested with:
>
> # cat ioctl.c
> #include <unistd.h>
> #include <sys/ioctl.h>
>
> int main(void)
> {
> int ret;
>
> ret = ioctl(STDOUT_FILENO, TCGETS, 0xdeadbeef);
> ret = ioctl(STDOUT_FILENO, TCSETSF, 0xdeadbeef);
> return 0;
> }
> # QEMU_STRACE= ./ioctl
> ...
> 578 ioctl(1,TCGETS,0xdeadbeef) = -1 errno=2 (Bad address)
> 578 ioctl(1,TCSETSF,0xdeadbeef) = -1 errno=2 (Bad address)
> ...
> # QEMU_STRACE= passwd
> ...
> 623 ioctl(0,TCGETS,0x3fffed04) = 0 ({})
> 623 ioctl(0,TCSETSF,{}) = 0
> ...
>
> Reported-by: Peter Maydell <peter.mayd...@linaro.org>
> Fixes: 79482e5987c8 ("linux-user: Add strace support for printing arguments
> of ioctl()")
> Signed-off-by: Laurent Vivier <laur...@vivier.eu>
> ---
> linux-user/strace.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index 5235b2260cdd..39554d903911 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -889,8 +889,12 @@ print_syscall_ret_ioctl(const struct syscallname *name,
> abi_long ret,
> arg_type++;
> target_size = thunk_type_size(arg_type, 0);
> argptr = lock_user(VERIFY_READ, arg2, target_size, 1);
> - thunk_print(argptr, arg_type);
> - unlock_user(argptr, arg2, target_size);
> + if (argptr) {
> + thunk_print(argptr, arg_type);
> + unlock_user(argptr, arg2, target_size);
> + } else {
> + print_pointer(arg2, 1);
> + }
> qemu_log(")");
> }
> }
> @@ -3119,8 +3123,12 @@ print_ioctl(const struct syscallname *name,
> arg_type++;
> target_size = thunk_type_size(arg_type, 0);
> argptr = lock_user(VERIFY_READ, arg2, target_size, 1);
> - thunk_print(argptr, arg_type);
> - unlock_user(argptr, arg2, target_size);
> + if (argptr) {
> + thunk_print(argptr, arg_type);
> + unlock_user(argptr, arg2, target_size);
> + } else {
> + print_pointer(arg2, 1);
> + }
> break;
> }
> break;
>
Applied to my linux-user-for-5.1 branch.
Thanks,
Laurent
