On Sat, 4 Jul 2020 at 17:36, Laurent Vivier <[email protected]> wrote:
>
> From: Filip Bozuta <[email protected]>
>
> This patch implements functionality for strace argument printing for ioctls.
Hi; Coverity points out some issues in this change:
> +#ifdef TARGET_NR_ioctl
> +static void
> +print_syscall_ret_ioctl(const struct syscallname *name, abi_long ret,
> + abi_long arg0, abi_long arg1, abi_long arg2,
> + abi_long arg3, abi_long arg4, abi_long arg5)
> +{
> + print_syscall_err(ret);
> +
> + if (ret >= 0) {
> + qemu_log(TARGET_ABI_FMT_ld, ret);
> +
> + const IOCTLEntry *ie;
> + const argtype *arg_type;
> + void *argptr;
> + int target_size;
> +
> + for (ie = ioctl_entries; ie->target_cmd != 0; ie++) {
> + if (ie->target_cmd == arg1) {
> + break;
> + }
> + }
> +
> + if (ie->target_cmd == arg1 &&
> + (ie->access == IOC_R || ie->access == IOC_RW)) {
> + arg_type = ie->arg_type;
> + qemu_log(" (");
> + arg_type++;
> + target_size = thunk_type_size(arg_type, 0);
> + argptr = lock_user(VERIFY_READ, arg2, target_size, 1);
Here we fail to check that lock_user() didn't return NULL...
> + thunk_print(argptr, arg_type);
...which would cause a segfault in thunk_print().
This is CID 1430271.
> + unlock_user(argptr, arg2, target_size);
> + qemu_log(")");
> + }
> + }
> + qemu_log("\n");
> +}
> +#endif
> +#ifdef TARGET_NR_ioctl
> +static void
> +print_ioctl(const struct syscallname *name,
> + abi_long arg0, abi_long arg1, abi_long arg2,
> + abi_long arg3, abi_long arg4, abi_long arg5)
> +{
> + case TYPE_PTR:
> + switch (ie->access) {
> + case IOC_R:
> + print_pointer(arg2, 1);
> + break;
> + case IOC_W:
> + case IOC_RW:
> + arg_type++;
> + target_size = thunk_type_size(arg_type, 0);
> + argptr = lock_user(VERIFY_READ, arg2, target_size, 1);
> + thunk_print(argptr, arg_type);
Similarly here we need to check that lock_user didn't fail.
This is CID 1430272.
> + unlock_user(argptr, arg2, target_size);
> + break;
> + }
> + break;
> + default:
> + g_assert_not_reached();
> + }
> + }
> + }
> + print_syscall_epilogue(name);
> +}
thanks
-- PMM
