Re: [PATCH 2/3] linux-user: Fix unlock_user API usage

[Nicholas Piggin <npiggin@gmail.com>]

On Wed, Mar 25, 2026 at 11:46:45AM +1000, Alistair Francis wrote:
> On Sat, Mar 21, 2026 at 10:50 AM Nicholas Piggin <npiggin@gmail.com> wrote:
> >
> > Fix errors in unlock_user() calls:
> > - unlock_user() with len=1 instead of len=written
> > - unlock_user() with len=1 instead of len=0
> > - unlock_user() with len=0 instead of len=1
> 
> Can you explain what the errors are and why this fixes them?
> 
> At least to me (I don't know the linux-user stuff very well) I don't
> understand what is being fixed here

Yeah I should explain a bit better in the changelog.

When CONFIG_DEBUG_REMAP=n, lock_user just validates the guest
pointer is within the guest address space and gives you a pointer
to host address that can be used with __get_user() etc. And
unlock_user does nothing, which is why it accumulated all these
errors.

With CONFIG_DEBUG_REMAP=y, they allocate a separate area of
memory for the host to work in, and that's the host pointer you
get to work with. The lock is supposed to say if it reads the
memory and if so it is copied in to your temp area. The unlock
says the length written to and it copies it out from temp area
back to guest image. End result is guest memory updates from
syscalls don't take effect.

The APIs are a bit clunky, might be able to be improved with
minor changes. Biggest help would be to run DEBUG_REMAP=y from
time to time in CI.

Thanks,
Nick

> 
> Alistair
> 
> >
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >  linux-user/linuxload.c | 2 +-
> >  linux-user/syscall.c   | 6 +++---
> >  2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/linux-user/linuxload.c b/linux-user/linuxload.c
> > index 85d700953e..79416a94c9 100644
> > --- a/linux-user/linuxload.c
> > +++ b/linux-user/linuxload.c
> > @@ -19,7 +19,7 @@ abi_long memcpy_to_target(abi_ulong dest, const void *src, unsigned long len)
> >          return -TARGET_EFAULT;
> >      }
> >      memcpy(host_ptr, src, len);
> > -    unlock_user(host_ptr, dest, 1);
> > +    unlock_user(host_ptr, dest, len);
> >      return 0;
> >  }
> >
> > diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> > index 7832a1aba5..13b8bd9ed3 100644
> > --- a/linux-user/syscall.c
> > +++ b/linux-user/syscall.c
> > @@ -2989,7 +2989,7 @@ get_timeout:
> >              if (put_user_u32(lv, optlen)) {
> >                  return -TARGET_EFAULT;
> >              }
> > -            unlock_user(results, optval_addr, 0);
> > +            unlock_user(results, optval_addr, len);
> >              break;
> >          }
> >  #endif /* LINUX_VERSION_CODE >= KERNEL_VERSION(4, 2, 0) */
> > @@ -4006,7 +4006,7 @@ static inline abi_long host_to_target_semarray(int semid, abi_ulong target_addr,
> >          __put_user((*host_array)[i], &array[i]);
> >      }
> >      g_free(*host_array);
> > -    unlock_user(array, target_addr, 1);
> > +    unlock_user(array, target_addr, nsems * sizeof(unsigned short));
> >
> >      return 0;
> >  }
> > @@ -7888,7 +7888,7 @@ static inline abi_long target_to_host_sigevent(struct sigevent *host_sevp,
> >      host_sevp->sigev_notify = tswap32(target_sevp->sigev_notify);
> >      host_sevp->sigev_notify_thread_id = tswap32(target_sevp->_sigev_un._tid);
> >
> > -    unlock_user_struct(target_sevp, target_addr, 1);
> > +    unlock_user_struct(target_sevp, target_addr, 0);
> >      return 0;
> >  }
> >
> > --
> > 2.51.0
> >
> >
>