On 12 July 2014 16:13, Joakim Tjernlund <joakim.tjernl...@transmode.se> wrote:
> Peter Maydell <peter.mayd...@linaro.org> 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
