Re: [PATCH v4] linux-user: Add most IFTUN ioctls

[Laurent Vivier]

Le 29/09/2020 à 03:48, Shu-Chun Weng via 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 <jkz@google.com>
> Signed-off-by: Shu-Chun Weng <scw@google.com>
> ---
> v2->v3:
>   IOCTL_SPECIAL(TUNSETTXFILTER) type changed to MK_PTR(TYPE_PTRVOID) for 
> strace
>   to display the raw pointer.
> 
>   Updated do_ioctl_TUNSETTXFILTER for correct usages of unlock_user() and
>   offsetof().
> 
> v3->v4:
>   IOCTL_SPECIAL(TUNSETTXFILTER) corrected to TYPE_PTRVOID.
> 
>  linux-user/ioctls.h       | 46 +++++++++++++++++++++++++++++++++++++++
>  linux-user/syscall.c      | 38 ++++++++++++++++++++++++++++++++
>  linux-user/syscall_defs.h | 32 +++++++++++++++++++++++++++
>  3 files changed, 116 insertions(+)
> 
...
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 945fc25279..1c955bc675 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
> @@ -5415,6 +5416,43 @@ 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(*target_filter), 1);
> +    if (!target_filter) {
> +        return -TARGET_EFAULT;
> +    }
> +    filter->flags = tswap16(target_filter->flags);
> +    filter->count = tswap16(target_filter->count);
> +    unlock_user(target_filter, arg, 0);
> +
> +    if (filter->count) {
> +        if (offsetof(struct tun_filter, addr) + filter->count * ETH_ALEN >
> +            MAX_STRUCT_SIZE) {
> +            return -TARGET_EFAULT;
> +        }
> +
> +        target_addr = lock_user(VERIFY_READ,
> +                                arg + 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 + offsetof(struct tun_filter, addr),
> +                    filter->count * ETH_ALEN);

As we don't modify target_addr memory content (locked with VERIFY_READ),
we can replace "filter->count * ETH_ALEN" by 0.

With that modified:

Reviewed-by: Laurent Vivier <laurent@vivier.eu>