Re: [PATCH 2/6] bsd-user/freebsd/os-syscall.c: unlock_iovec

[Richard Henderson]

On 6/7/22 13:14, Warner Losh wrote:
> +void unlock_iovec(struct iovec *vec, abi_ulong target_addr,
> +        int count, int copy)
> +{
> +    struct target_iovec *target_vec;
> +
> +    target_vec = lock_user(VERIFY_READ, target_addr,
> +                           count * sizeof(struct target_iovec), 1);
> +    if (target_vec) {

Locking the same region twice seems like a bad idea.

How about something like

typedef struct {
    struct target_iovec *target;
    abi_ulong target_addr;
    int count;
    struct iovec host[];
} IOVecMap;

IOVecMap *lock_iovec(abi_ulong target_addr, int count, bool copy_in)
{
    IOVecMap *map;

    if (count == 0) ...
    if (count < 0) ...

    map = g_try_malloc0(sizeof(IOVecNew) + sizeof(struct iovec) * count);
    if (!map) ...

    map->target = lock_user(...);
    if (!map->target) {
        g_free(map);
        errno = EFAULT;
        return NULL;
    }
    map->target_addr = target_addr;
    map->count = count;

    // lock loop

 fail:
    unlock_iovec(vec, false);
    errno = err;
    return NULL;
}

void unlock_iovec(IOVecMap *map, bool copy_out)
{
    for (int i = 0, count = map->count; i < count; ++i) {
        if (map->host[i].iov_base) {
            abi_ulong target_base = tswapal(map->target[i].iov_base);
            unlock_user(map->host[i].iov_base, target_base,
                        copy_out ? map->host[i].iov_len : 0);
        }
    }
    unlock_user(map->target, map->target_addr, 0);
    g_free(map);
}


r~