Le 24/07/2020 à 01:10, Shu-Chun Weng a écrit :
> The three options handling `struct sock_fprog` (TUNATTACHFILTER,
> TUNDETACHFILTER, and TUNGETFILTER) are not implemented. Linux kernel
> keeps a user space pointer in them which we cannot correctly handle.
>
> Signed-off-by: Josh Kunz <j...@google.com>
> Signed-off-by: Shu-Chun Weng <s...@google.com>
> ---
> v2:
> Title changed from "linux-user: Add several IFTUN ioctls"
>
> Properly specify the argument types for various options, including a custom
> implementation for TUNSETTXFILTER.
>
> #ifdef guards for macros introduced up to 5 years ago.
>
> linux-user/ioctls.h | 45 +++++++++++++++++++++++++++++++++++++++
> linux-user/syscall.c | 36 +++++++++++++++++++++++++++++++
> linux-user/syscall_defs.h | 32 ++++++++++++++++++++++++++++
> 3 files changed, 113 insertions(+)
>
> diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
> index 0713ae1311..b9fb01f558 100644
> --- a/linux-user/ioctls.h
> +++ b/linux-user/ioctls.h
> @@ -593,3 +593,48 @@
> IOCTL(KCOV_DISABLE, 0, TYPE_NULL)
> IOCTL(KCOV_INIT_TRACE, IOC_R, TYPE_ULONG)
> #endif
> +
> + IOCTL(TUNSETDEBUG, IOC_W, TYPE_INT)
> + IOCTL(TUNSETIFF, IOC_RW, MK_PTR(MK_STRUCT(STRUCT_short_ifreq)))
Why is this IOC_RW?
> + IOCTL(TUNSETPERSIST, IOC_W, TYPE_INT)
> + IOCTL(TUNSETOWNER, IOC_W, TYPE_INT)
> + IOCTL(TUNSETLINK, IOC_W, TYPE_INT)
> + IOCTL(TUNSETGROUP, IOC_W, TYPE_INT)
> + IOCTL(TUNGETFEATURES, IOC_R, MK_PTR(TYPE_INT))
> + IOCTL(TUNSETOFFLOAD, IOC_W, TYPE_LONG)
Why is this long?
> + IOCTL_SPECIAL(TUNSETTXFILTER, IOC_W, do_ioctl_TUNSETTXFILTER,
> + /*
> + * We can't represent `struct tun_filter` in thunk so leaving
> + * this empty. do_ioctl_TUNSETTXFILTER will do the
> conversion.
> + */
> + TYPE_NULL)
You should use TYPE_PTRVOID to allow QEMU_STRACE to display the pointer.
Or implement the function to dump the structure (see STRUCT_termios and
struct_termios_def).
> + IOCTL(TUNGETIFF, IOC_R, MK_PTR(MK_STRUCT(STRUCT_short_ifreq)))
> + IOCTL(TUNGETSNDBUF, IOC_R, MK_PTR(TYPE_INT))
> + IOCTL(TUNSETSNDBUF, IOC_W, MK_PTR(TYPE_INT))
> + /*
> + * TUNATTACHFILTER and TUNDETACHFILTER are not supported. Linux kernel
> keeps a
> + * user pointer in TUNATTACHFILTER, which we are not able to correctly
> handle.
> + */
> + IOCTL(TUNGETVNETHDRSZ, IOC_R, MK_PTR(TYPE_INT))
> + IOCTL(TUNSETVNETHDRSZ, IOC_W, MK_PTR(TYPE_INT))
> + IOCTL(TUNSETQUEUE, IOC_W, MK_PTR(MK_STRUCT(STRUCT_short_ifreq)))
> + IOCTL(TUNSETIFINDEX , IOC_W, MK_PTR(TYPE_INT))
> + /* TUNGETFILTER is not supported: see TUNATTACHFILTER. */
> + IOCTL(TUNSETVNETLE, IOC_W, MK_PTR(TYPE_INT))
> + IOCTL(TUNGETVNETLE, IOC_R, MK_PTR(TYPE_INT))
> +#ifdef TUNSETVNETBE
> + IOCTL(TUNSETVNETBE, IOC_W, MK_PTR(TYPE_INT))
> + IOCTL(TUNGETVNETBE, IOC_R, MK_PTR(TYPE_INT))
> +#endif
> +#ifdef TUNSETSTEERINGEBPF
> + IOCTL(TUNSETSTEERINGEBPF, IOC_W, MK_PTR(TYPE_INT))
> +#endif
> +#ifdef TUNSETFILTEREBPF
> + IOCTL(TUNSETFILTEREBPF, IOC_W, MK_PTR(TYPE_INT))
> +#endif
> +#ifdef TUNSETCARRIER
> + IOCTL(TUNSETCARRIER, IOC_W, MK_PTR(TYPE_INT))
> +#endif
> +#ifdef TUNGETDEVNETNS
> + IOCTL(TUNGETDEVNETNS, IOC_R, TYPE_NULL)
> +#endif
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 1211e759c2..7f1efed189 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -56,6 +56,7 @@
> #include <linux/wireless.h>
> #include <linux/icmp.h>
> #include <linux/icmpv6.h>
> +#include <linux/if_tun.h>
> #include <linux/errqueue.h>
> #include <linux/random.h>
> #ifdef CONFIG_TIMERFD
> @@ -5422,6 +5423,41 @@ static abi_long do_ioctl_drm(const IOCTLEntry *ie,
> uint8_t *buf_temp,
>
> #endif
>
> +static abi_long do_ioctl_TUNSETTXFILTER(const IOCTLEntry *ie, uint8_t
> *buf_temp,
> + int fd, int cmd, abi_long arg)
> +{
> + struct tun_filter *filter = (struct tun_filter *)buf_temp;
> + struct tun_filter *target_filter;
> + char *target_addr;
> +
> + assert(ie->access == IOC_W);
> +
> + target_filter = lock_user(VERIFY_READ, arg, sizeof(*filter), 1);
sizeof(*target_filter) vould be more coherent: we lock the target memory
so use the size of the type of the target.
> + if (!target_filter) {
> + return -TARGET_EFAULT;
> + }
> + filter->flags = tswap16(target_filter->flags);
> + filter->count = tswap16(target_filter->count);
> + unlock_user(target_filter, arg, sizeof(*filter));
unlock_user(target_filter, arg, 0) as we don't need to copy the value
back to the target.
> +
> + if (filter->count) {
> + if (sizeof(*filter) + filter->count * ETH_ALEN > MAX_STRUCT_SIZE) {
Rather than sizeof() use offsetof(struct tun_filter, addr)
> + return -TARGET_EFAULT;
> + }
> +
> + target_addr = lock_user(VERIFY_READ, arg + sizeof(*filter),
Rather than sizeof() use offsetof(struct tun_filter, addr)
> + filter->count * ETH_ALEN, 1);
> + if (!target_addr) {
> + return -TARGET_EFAULT;
> + }
> + memcpy(filter->addr, target_addr, filter->count * ETH_ALEN);
> + unlock_user(target_addr, arg + sizeof(*filter),
offsetof(struct tun_filter, addr)
Thanks,
Laurent
