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.
We unlock the iovec memory in the lock_iovec()
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);
}
And wouldn't we want to filter out the iov_base that == 0 since
we may terminate the loop before we get to the count. When the
I/O is done, we'll call it not with the number we mapped, but with
the original number... Or am I not understanding something here...
Warner
}
unlock_user(map->target, map->target_addr, 0);
g_free(map);
}
r~