* [PATCH 0/3] Fix missing and incorrect unlock_user calls
@ 2026-03-21 0:48 Nicholas Piggin
2026-03-21 0:48 ` [PATCH 1/3] bsd-user: Fix unlock_user API usage Nicholas Piggin
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Nicholas Piggin @ 2026-03-21 0:48 UTC (permalink / raw)
To: qemu-devel
Cc: Nicholas Piggin, Warner Losh, Kyle Evans, Laurent Vivier,
Pierrick Bouvier, Palmer Dabbelt, Alistair Francis, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, qemu-riscv
This fixes a few tree-wide issues I found when implementing
vector state signal handling for riscv.
Thanks,
Nick
Nicholas Piggin (3):
bsd-user: Fix unlock_user API usage
linux-user: Fix unlock_user API usage
linux-user: riscv: Fix signal frame user mapping
bsd-user/bsd-misc.c | 2 +-
bsd-user/bsd-misc.h | 2 +-
bsd-user/bsdload.c | 2 +-
linux-user/linuxload.c | 2 +-
linux-user/riscv/signal.c | 1 +
linux-user/syscall.c | 6 +++---
6 files changed, 8 insertions(+), 7 deletions(-)
--
2.51.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/3] bsd-user: Fix unlock_user API usage
2026-03-21 0:48 [PATCH 0/3] Fix missing and incorrect unlock_user calls Nicholas Piggin
@ 2026-03-21 0:48 ` Nicholas Piggin
2026-03-21 4:17 ` Warner Losh
2026-03-21 0:48 ` [PATCH 2/3] linux-user: " Nicholas Piggin
2026-03-21 0:48 ` [PATCH 3/3] linux-user: riscv: Fix signal frame user mapping Nicholas Piggin
2 siblings, 1 reply; 11+ messages in thread
From: Nicholas Piggin @ 2026-03-21 0:48 UTC (permalink / raw)
To: qemu-devel
Cc: Nicholas Piggin, Warner Losh, Kyle Evans, Laurent Vivier,
Pierrick Bouvier, Palmer Dabbelt, Alistair Francis, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, qemu-riscv
Fix errors in unlock_user() calls:
- unlock_user() with len=1 instead of len=written
- unlock_user() with len=1 instead of len=0
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
bsd-user/bsd-misc.c | 2 +-
bsd-user/bsd-misc.h | 2 +-
bsd-user/bsdload.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/bsd-user/bsd-misc.c b/bsd-user/bsd-misc.c
index 3e1968718f..eac3edb8ac 100644
--- a/bsd-user/bsd-misc.c
+++ b/bsd-user/bsd-misc.c
@@ -95,7 +95,7 @@ abi_long host_to_target_semarray(int semid, abi_ulong target_addr,
for (i = 0; i < nsems; i++) {
__put_user(array[i], host_array + i);
}
- unlock_user(array, target_addr, 1);
+ unlock_user(array, target_addr, nsems * sizeof(unsigned short));
return 0;
}
diff --git a/bsd-user/bsd-misc.h b/bsd-user/bsd-misc.h
index 13e40d9cde..13abc7e3b1 100644
--- a/bsd-user/bsd-misc.h
+++ b/bsd-user/bsd-misc.h
@@ -211,7 +211,7 @@ static inline abi_long do_bsd___semctl(int semid, int semnum, int target_cmd,
break;
}
out:
- unlock_user(target_un, un_ptr, 1);
+ unlock_user(target_un, un_ptr, 0);
return ret;
}
diff --git a/bsd-user/bsdload.c b/bsd-user/bsdload.c
index 5b3c061a45..9ea3b93825 100644
--- a/bsd-user/bsdload.c
+++ b/bsd-user/bsdload.c
@@ -30,7 +30,7 @@ abi_long memcpy_to_target(abi_ulong dest, const void *src,
return -TARGET_EFAULT;
}
memcpy(host_ptr, src, len);
- unlock_user(host_ptr, dest, 1);
+ unlock_user(host_ptr, dest, len);
return 0;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/3] linux-user: Fix unlock_user API usage
2026-03-21 0:48 [PATCH 0/3] Fix missing and incorrect unlock_user calls Nicholas Piggin
2026-03-21 0:48 ` [PATCH 1/3] bsd-user: Fix unlock_user API usage Nicholas Piggin
@ 2026-03-21 0:48 ` Nicholas Piggin
2026-03-22 6:54 ` Chao Liu
2026-03-25 1:46 ` Alistair Francis
2026-03-21 0:48 ` [PATCH 3/3] linux-user: riscv: Fix signal frame user mapping Nicholas Piggin
2 siblings, 2 replies; 11+ messages in thread
From: Nicholas Piggin @ 2026-03-21 0:48 UTC (permalink / raw)
To: qemu-devel
Cc: Nicholas Piggin, Warner Losh, Kyle Evans, Laurent Vivier,
Pierrick Bouvier, Palmer Dabbelt, Alistair Francis, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, qemu-riscv
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
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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/3] linux-user: riscv: Fix signal frame user mapping
2026-03-21 0:48 [PATCH 0/3] Fix missing and incorrect unlock_user calls Nicholas Piggin
2026-03-21 0:48 ` [PATCH 1/3] bsd-user: Fix unlock_user API usage Nicholas Piggin
2026-03-21 0:48 ` [PATCH 2/3] linux-user: " Nicholas Piggin
@ 2026-03-21 0:48 ` Nicholas Piggin
2026-03-22 6:44 ` Chao Liu
2026-03-25 1:53 ` Alistair Francis
2 siblings, 2 replies; 11+ messages in thread
From: Nicholas Piggin @ 2026-03-21 0:48 UTC (permalink / raw)
To: qemu-devel
Cc: Nicholas Piggin, Warner Losh, Kyle Evans, Laurent Vivier,
Pierrick Bouvier, Palmer Dabbelt, Alistair Francis, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, qemu-riscv
riscv signal frame setup is missing unlock_user_struct().
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
linux-user/riscv/signal.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/linux-user/riscv/signal.c b/linux-user/riscv/signal.c
index 358fa1d82d..22b1b8149f 100644
--- a/linux-user/riscv/signal.c
+++ b/linux-user/riscv/signal.c
@@ -126,6 +126,7 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
setup_ucontext(&frame->uc, env, set);
frame->info = *info;
+ unlock_user_struct(frame, frame_addr, 1);
env->pc = ka->_sa_handler;
env->gpr[xSP] = frame_addr;
--
2.51.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] bsd-user: Fix unlock_user API usage
2026-03-21 0:48 ` [PATCH 1/3] bsd-user: Fix unlock_user API usage Nicholas Piggin
@ 2026-03-21 4:17 ` Warner Losh
0 siblings, 0 replies; 11+ messages in thread
From: Warner Losh @ 2026-03-21 4:17 UTC (permalink / raw)
To: Nicholas Piggin
Cc: qemu-devel, Kyle Evans, Laurent Vivier, Pierrick Bouvier,
Palmer Dabbelt, Alistair Francis, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, qemu-riscv
[-- Attachment #1: Type: text/plain, Size: 1828 bytes --]
On Fri, Mar 20, 2026 at 6:49 PM 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
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> bsd-user/bsd-misc.c | 2 +-
> bsd-user/bsd-misc.h | 2 +-
> bsd-user/bsdload.c | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
Reviewed-by: Warner Losh <imp@bsdimp.com>
> diff --git a/bsd-user/bsd-misc.c b/bsd-user/bsd-misc.c
> index 3e1968718f..eac3edb8ac 100644
> --- a/bsd-user/bsd-misc.c
> +++ b/bsd-user/bsd-misc.c
> @@ -95,7 +95,7 @@ abi_long host_to_target_semarray(int semid, abi_ulong
> target_addr,
> for (i = 0; i < nsems; i++) {
> __put_user(array[i], host_array + i);
> }
> - unlock_user(array, target_addr, 1);
> + unlock_user(array, target_addr, nsems * sizeof(unsigned short));
> return 0;
> }
>
> diff --git a/bsd-user/bsd-misc.h b/bsd-user/bsd-misc.h
> index 13e40d9cde..13abc7e3b1 100644
> --- a/bsd-user/bsd-misc.h
> +++ b/bsd-user/bsd-misc.h
> @@ -211,7 +211,7 @@ static inline abi_long do_bsd___semctl(int semid, int
> semnum, int target_cmd,
> break;
> }
> out:
> - unlock_user(target_un, un_ptr, 1);
> + unlock_user(target_un, un_ptr, 0);
> return ret;
> }
>
> diff --git a/bsd-user/bsdload.c b/bsd-user/bsdload.c
> index 5b3c061a45..9ea3b93825 100644
> --- a/bsd-user/bsdload.c
> +++ b/bsd-user/bsdload.c
> @@ -30,7 +30,7 @@ abi_long memcpy_to_target(abi_ulong dest, const void
> *src,
> return -TARGET_EFAULT;
> }
> memcpy(host_ptr, src, len);
> - unlock_user(host_ptr, dest, 1);
> + unlock_user(host_ptr, dest, len);
> return 0;
> }
>
> --
> 2.51.0
>
>
[-- Attachment #2: Type: text/html, Size: 2591 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] linux-user: riscv: Fix signal frame user mapping
2026-03-21 0:48 ` [PATCH 3/3] linux-user: riscv: Fix signal frame user mapping Nicholas Piggin
@ 2026-03-22 6:44 ` Chao Liu
2026-03-25 1:53 ` Alistair Francis
1 sibling, 0 replies; 11+ messages in thread
From: Chao Liu @ 2026-03-22 6:44 UTC (permalink / raw)
To: Nicholas Piggin
Cc: qemu-devel, Warner Losh, Kyle Evans, Laurent Vivier,
Pierrick Bouvier, Palmer Dabbelt, Alistair Francis, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, qemu-riscv
On Sat, Mar 21, 2026 at 10:48:36AM +1000, Nicholas Piggin wrote:
> riscv signal frame setup is missing unlock_user_struct().
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Reviewed-by: Chao Liu <chao.liu.zevorn@gmail.com>
Thanks,
Chao
> ---
> linux-user/riscv/signal.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/linux-user/riscv/signal.c b/linux-user/riscv/signal.c
> index 358fa1d82d..22b1b8149f 100644
> --- a/linux-user/riscv/signal.c
> +++ b/linux-user/riscv/signal.c
> @@ -126,6 +126,7 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
>
> setup_ucontext(&frame->uc, env, set);
> frame->info = *info;
> + unlock_user_struct(frame, frame_addr, 1);
>
> env->pc = ka->_sa_handler;
> env->gpr[xSP] = frame_addr;
> --
> 2.51.0
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] linux-user: Fix unlock_user API usage
2026-03-21 0:48 ` [PATCH 2/3] linux-user: " Nicholas Piggin
@ 2026-03-22 6:54 ` Chao Liu
2026-03-26 6:01 ` Nicholas Piggin
2026-03-25 1:46 ` Alistair Francis
1 sibling, 1 reply; 11+ messages in thread
From: Chao Liu @ 2026-03-22 6:54 UTC (permalink / raw)
To: Nicholas Piggin
Cc: qemu-devel, Warner Losh, Kyle Evans, Laurent Vivier,
Pierrick Bouvier, Palmer Dabbelt, Alistair Francis, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, qemu-riscv
On Sat, Mar 21, 2026 at 10:48:35AM +1000, Nicholas Piggin 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
>
> 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);
This fixes the writeback -- old code had 0 which meant
no flush at all, so getsockopt results were silently
lost under CONFIG_DEBUG_REMAP.
Minor observation: getsockopt returns actual bytes in
lv, so using lv rather than len would be more precise.
That said, the swap loop above also uses len, so this
is consistent with the existing code.
> 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);
Right. This function only reads from target, so
copy=0 (no writeback) is the correct semantic.
Reviewed-by: Chao Liu <chao.liu.zevorn@gmail.com>
Thanks,
Chao
> return 0;
> }
>
> --
> 2.51.0
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] linux-user: Fix unlock_user API usage
2026-03-21 0:48 ` [PATCH 2/3] linux-user: " Nicholas Piggin
2026-03-22 6:54 ` Chao Liu
@ 2026-03-25 1:46 ` Alistair Francis
2026-03-26 6:20 ` Nicholas Piggin
1 sibling, 1 reply; 11+ messages in thread
From: Alistair Francis @ 2026-03-25 1:46 UTC (permalink / raw)
To: Nicholas Piggin
Cc: qemu-devel, Warner Losh, Kyle Evans, Laurent Vivier,
Pierrick Bouvier, Palmer Dabbelt, Alistair Francis, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, qemu-riscv
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
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
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] linux-user: riscv: Fix signal frame user mapping
2026-03-21 0:48 ` [PATCH 3/3] linux-user: riscv: Fix signal frame user mapping Nicholas Piggin
2026-03-22 6:44 ` Chao Liu
@ 2026-03-25 1:53 ` Alistair Francis
1 sibling, 0 replies; 11+ messages in thread
From: Alistair Francis @ 2026-03-25 1:53 UTC (permalink / raw)
To: Nicholas Piggin
Cc: qemu-devel, Warner Losh, Kyle Evans, Laurent Vivier,
Pierrick Bouvier, Palmer Dabbelt, Alistair Francis, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, qemu-riscv
On Sat, Mar 21, 2026 at 10:50 AM Nicholas Piggin <npiggin@gmail.com> wrote:
>
> riscv signal frame setup is missing unlock_user_struct().
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> linux-user/riscv/signal.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/linux-user/riscv/signal.c b/linux-user/riscv/signal.c
> index 358fa1d82d..22b1b8149f 100644
> --- a/linux-user/riscv/signal.c
> +++ b/linux-user/riscv/signal.c
> @@ -126,6 +126,7 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
>
> setup_ucontext(&frame->uc, env, set);
> frame->info = *info;
> + unlock_user_struct(frame, frame_addr, 1);
>
> env->pc = ka->_sa_handler;
> env->gpr[xSP] = frame_addr;
> --
> 2.51.0
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] linux-user: Fix unlock_user API usage
2026-03-22 6:54 ` Chao Liu
@ 2026-03-26 6:01 ` Nicholas Piggin
0 siblings, 0 replies; 11+ messages in thread
From: Nicholas Piggin @ 2026-03-26 6:01 UTC (permalink / raw)
To: Chao Liu
Cc: qemu-devel, Warner Losh, Kyle Evans, Laurent Vivier,
Pierrick Bouvier, Palmer Dabbelt, Alistair Francis, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, qemu-riscv
On Sun, Mar 22, 2026 at 02:54:31PM +0800, Chao Liu wrote:
> On Sat, Mar 21, 2026 at 10:48:35AM +1000, Nicholas Piggin 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
> >
> > 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);
> This fixes the writeback -- old code had 0 which meant
> no flush at all, so getsockopt results were silently
> lost under CONFIG_DEBUG_REMAP.
>
> Minor observation: getsockopt returns actual bytes in
> lv, so using lv rather than len would be more precise.
> That said, the swap loop above also uses len, so this
> is consistent with the existing code.
I guess this comment is for the next hunk below.
The swap loop as you noted stores to the full len, so I
think it's better to use that. If the logic changes to
only store lv, then the unlock would change.
> > 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;
> > }
Actually missing unlock on error too, I'll fix that up.
> > #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);
> Right. This function only reads from target, so
> copy=0 (no writeback) is the correct semantic.
>
> Reviewed-by: Chao Liu <chao.liu.zevorn@gmail.com>
Thanks,
Nick
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] linux-user: Fix unlock_user API usage
2026-03-25 1:46 ` Alistair Francis
@ 2026-03-26 6:20 ` Nicholas Piggin
0 siblings, 0 replies; 11+ messages in thread
From: Nicholas Piggin @ 2026-03-26 6:20 UTC (permalink / raw)
To: Alistair Francis
Cc: qemu-devel, Warner Losh, Kyle Evans, Laurent Vivier,
Pierrick Bouvier, Palmer Dabbelt, Alistair Francis, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, qemu-riscv
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
> >
> >
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-03-26 6:21 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-21 0:48 [PATCH 0/3] Fix missing and incorrect unlock_user calls Nicholas Piggin
2026-03-21 0:48 ` [PATCH 1/3] bsd-user: Fix unlock_user API usage Nicholas Piggin
2026-03-21 4:17 ` Warner Losh
2026-03-21 0:48 ` [PATCH 2/3] linux-user: " Nicholas Piggin
2026-03-22 6:54 ` Chao Liu
2026-03-26 6:01 ` Nicholas Piggin
2026-03-25 1:46 ` Alistair Francis
2026-03-26 6:20 ` Nicholas Piggin
2026-03-21 0:48 ` [PATCH 3/3] linux-user: riscv: Fix signal frame user mapping Nicholas Piggin
2026-03-22 6:44 ` Chao Liu
2026-03-25 1:53 ` Alistair Francis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox