On Tue, Jun 7, 2022 at 2:28 PM Richard Henderson <
richard.hender...@linaro.org> wrote:
> 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~
>
