Re: [Qemu-devel] [PATCH 1/4] qemu-user: Impl. setsockopt(SO_BINDTODEVICE)

[Peter Maydell]

On 12 July 2014 16:13, Joakim Tjernlund <address@hidden> wrote:
> Peter Maydell <address@hidden> wrote on 2014/07/11 19:02:30:
>> >
>> > hmm, should we not pass through as is to the kernel?
>> > Since we don't copy anything we could just remove this
>> > check and let the kernel decide policy?
>>
>> I thought about that, but there's a corner case:
>> the kernel does the clamping of the optlen before the
>> copy_from_user(), which means if you have:
>>  [interface name] [unreadable memory]
>> and optlen is long enough that optval_addr + optlen
>> reaches into the unreadable memory, then QEMU will return
>> EFAULT (whereas the native kernel implementation will
>> succeed) unless we do the clamping of the optlen ourselves.
>
> hmm, this kernel code:
>         if (optlen > IFNAMSIZ - 1)
>                 optlen = IFNAMSIZ - 1;
>         memset(devname, 0, sizeof(devname));
>
>         ret = -EFAULT;
>         if (copy_from_user(devname, optval, optlen))
>                 goto out;
>
> suggests to me that the qemu code could be written:
>
>         case TARGET_SO_BINDTODEVICE:
>         {
>                 char *dev_ifname;
>
>                 if (optlen > IFNAMSIZ - 1) {
>                     optlen = IFNAMSIZ - 1;
>                 }
>                 dev_ifname = lock_user(VERIFY_READ, optval_addr, optlen,
> 1);
>                 if (!dev_ifname) {
>                     return -TARGET_EFAULT;
>                 }
>                 optname = SO_BINDTODEVICE;
>                 ret = get_errno(setsockopt(sockfd, level, optname,
> dev_ifname, optlen));
>                 unlock_user (dev_ifname, optval_addr, 0);
>                 return ret;
>         }
>
> Not a big deal, I just wanted to improve my understanding of the
> kernel/qemu code.

That would work with the current kernel implementation, but
the API definition (as described in the socket(7) manpage)
says we should pass a NUL-terminated string to the kernel,
so it's more robust for us to make sure we do so.

thanks
-- PMM