* [PATCH v3 0/2] linux-user: Fix unaligned memory access in prlimit64 syscall
@ 2023-02-24 0:39 Ilya Leoshkevich
2023-02-24 0:39 ` [PATCH v3 1/2] " Ilya Leoshkevich
2023-02-24 0:39 ` [PATCH v3 2/2] tests/tcg/linux-test: Add linux-fork-trap test Ilya Leoshkevich
0 siblings, 2 replies; 6+ messages in thread
From: Ilya Leoshkevich @ 2023-02-24 0:39 UTC (permalink / raw)
To: Alex Bennée, Laurent Vivier
Cc: qemu-devel, Christian Borntraeger, Ilya Leoshkevich
v2: https://lists.gnu.org/archive/html/qemu-devel/2023-02/msg07017.html
v2 -> v3: Fix __put_user() argument order mixup.
I tested v2 only against sh4, where the original problem was
discovered, but it regresses aarch64.
v1: https://lists.gnu.org/archive/html/qemu-devel/2023-02/msg06999.html
v1 -> v2: Fix by using proper target_rlimit64 alignment (Richard).
Use __get_user() and __put_user() (Philippe - if I understood
the suggestion correctly).
Hi,
Richard reported [1] that the new linux-fork-trap test was failing
under UBSan [2], so it was excluded from the PR.
This is a resend of the test plus the fix for the additional issue that
it uncovered.
[1] https://lists.gnu.org/archive/html/qemu-devel/2023-02/msg06130.html
[2] https://gitlab.com/qemu-project/qemu/-/jobs/3807471447#L5064
Best regards,
Ilya
Ilya Leoshkevich (2):
linux-user: Fix unaligned memory access in prlimit64 syscall
tests/tcg/linux-test: Add linux-fork-trap test
linux-user/generic/target_resource.h | 4 +-
linux-user/syscall.c | 8 ++--
tests/tcg/multiarch/linux/linux-fork-trap.c | 51 +++++++++++++++++++++
3 files changed, 57 insertions(+), 6 deletions(-)
create mode 100644 tests/tcg/multiarch/linux/linux-fork-trap.c
--
2.39.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 1/2] linux-user: Fix unaligned memory access in prlimit64 syscall
2023-02-24 0:39 [PATCH v3 0/2] linux-user: Fix unaligned memory access in prlimit64 syscall Ilya Leoshkevich
@ 2023-02-24 0:39 ` Ilya Leoshkevich
2023-02-24 7:42 ` Philippe Mathieu-Daudé
` (2 more replies)
2023-02-24 0:39 ` [PATCH v3 2/2] tests/tcg/linux-test: Add linux-fork-trap test Ilya Leoshkevich
1 sibling, 3 replies; 6+ messages in thread
From: Ilya Leoshkevich @ 2023-02-24 0:39 UTC (permalink / raw)
To: Alex Bennée, Laurent Vivier
Cc: qemu-devel, Christian Borntraeger, Ilya Leoshkevich, Richard Henderson
target_rlimit64 contains uint64_t fields, so it's 8-byte aligned on
some hosts, while some guests may align their respective type on a
4-byte boundary. This may lead to an unaligned access, which is an UB.
Fix by defining the fields as abi_ullong. This makes the host alignment
match that of the guest, and lets the compiler know that it should emit
code that can deal with the guest alignment.
While at it, also use __get_user() and __put_user() instead of
tswap64().
Fixes: 163a05a8398b ("linux-user: Implement prlimit64 syscall")
Reported-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
linux-user/generic/target_resource.h | 4 ++--
linux-user/syscall.c | 8 ++++----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/linux-user/generic/target_resource.h b/linux-user/generic/target_resource.h
index 539d8c46772..37d3eb09b3b 100644
--- a/linux-user/generic/target_resource.h
+++ b/linux-user/generic/target_resource.h
@@ -12,8 +12,8 @@ struct target_rlimit {
};
struct target_rlimit64 {
- uint64_t rlim_cur;
- uint64_t rlim_max;
+ abi_ullong rlim_cur;
+ abi_ullong rlim_max;
};
#define TARGET_RLIM_INFINITY ((abi_ulong)-1)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index a6c426d73cf..73082531ffc 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -12886,8 +12886,8 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
if (!lock_user_struct(VERIFY_READ, target_rnew, arg3, 1)) {
return -TARGET_EFAULT;
}
- rnew.rlim_cur = tswap64(target_rnew->rlim_cur);
- rnew.rlim_max = tswap64(target_rnew->rlim_max);
+ __get_user(rnew.rlim_cur, &target_rnew->rlim_cur);
+ __get_user(rnew.rlim_max, &target_rnew->rlim_max);
unlock_user_struct(target_rnew, arg3, 0);
rnewp = &rnew;
}
@@ -12897,8 +12897,8 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
if (!lock_user_struct(VERIFY_WRITE, target_rold, arg4, 1)) {
return -TARGET_EFAULT;
}
- target_rold->rlim_cur = tswap64(rold.rlim_cur);
- target_rold->rlim_max = tswap64(rold.rlim_max);
+ __put_user(rold.rlim_cur, &target_rold->rlim_cur);
+ __put_user(rold.rlim_max, &target_rold->rlim_max);
unlock_user_struct(target_rold, arg4, 1);
}
return ret;
--
2.39.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 2/2] tests/tcg/linux-test: Add linux-fork-trap test
2023-02-24 0:39 [PATCH v3 0/2] linux-user: Fix unaligned memory access in prlimit64 syscall Ilya Leoshkevich
2023-02-24 0:39 ` [PATCH v3 1/2] " Ilya Leoshkevich
@ 2023-02-24 0:39 ` Ilya Leoshkevich
1 sibling, 0 replies; 6+ messages in thread
From: Ilya Leoshkevich @ 2023-02-24 0:39 UTC (permalink / raw)
To: Alex Bennée, Laurent Vivier
Cc: qemu-devel, Christian Borntraeger, Ilya Leoshkevich, Richard Henderson
Check that dying due to a signal does not deadlock.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
tests/tcg/multiarch/linux/linux-fork-trap.c | 51 +++++++++++++++++++++
1 file changed, 51 insertions(+)
create mode 100644 tests/tcg/multiarch/linux/linux-fork-trap.c
diff --git a/tests/tcg/multiarch/linux/linux-fork-trap.c b/tests/tcg/multiarch/linux/linux-fork-trap.c
new file mode 100644
index 00000000000..2bfef800c3e
--- /dev/null
+++ b/tests/tcg/multiarch/linux/linux-fork-trap.c
@@ -0,0 +1,51 @@
+/*
+ * Test that a fork()ed process terminates after __builtin_trap().
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include <assert.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/resource.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+int main(void)
+{
+ struct rlimit nodump;
+ pid_t err, pid;
+ int wstatus;
+
+ pid = fork();
+ assert(pid != -1);
+ if (pid == 0) {
+ /* We are about to crash on purpose; disable core dumps. */
+ if (getrlimit(RLIMIT_CORE, &nodump)) {
+ return EXIT_FAILURE;
+ }
+ nodump.rlim_cur = 0;
+ if (setrlimit(RLIMIT_CORE, &nodump)) {
+ return EXIT_FAILURE;
+ }
+ /*
+ * An alternative would be to dereference a NULL pointer, but that
+ * would be an UB in C.
+ */
+ printf("about to trigger fault...\n");
+#if defined(__MICROBLAZE__)
+ /*
+ * gcc emits "bri 0", which is an endless loop.
+ * Take glibc's ABORT_INSTRUCTION.
+ */
+ asm volatile("brki r0,-1");
+#else
+ __builtin_trap();
+#endif
+ }
+ err = waitpid(pid, &wstatus, 0);
+ assert(err == pid);
+ assert(WIFSIGNALED(wstatus));
+ printf("faulting thread exited cleanly\n");
+
+ return EXIT_SUCCESS;
+}
--
2.39.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] linux-user: Fix unaligned memory access in prlimit64 syscall
2023-02-24 0:39 ` [PATCH v3 1/2] " Ilya Leoshkevich
@ 2023-02-24 7:42 ` Philippe Mathieu-Daudé
2023-03-06 21:34 ` Laurent Vivier
2023-03-07 10:32 ` Laurent Vivier
2 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-02-24 7:42 UTC (permalink / raw)
To: Ilya Leoshkevich, Alex Bennée, Laurent Vivier
Cc: qemu-devel, Christian Borntraeger, Richard Henderson
On 24/2/23 01:39, Ilya Leoshkevich wrote:
> target_rlimit64 contains uint64_t fields, so it's 8-byte aligned on
> some hosts, while some guests may align their respective type on a
> 4-byte boundary. This may lead to an unaligned access, which is an UB.
>
> Fix by defining the fields as abi_ullong. This makes the host alignment
> match that of the guest, and lets the compiler know that it should emit
> code that can deal with the guest alignment.
>
> While at it, also use __get_user() and __put_user() instead of
> tswap64().
>
> Fixes: 163a05a8398b ("linux-user: Implement prlimit64 syscall")
> Reported-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
> linux-user/generic/target_resource.h | 4 ++--
> linux-user/syscall.c | 8 ++++----
> 2 files changed, 6 insertions(+), 6 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] linux-user: Fix unaligned memory access in prlimit64 syscall
2023-02-24 0:39 ` [PATCH v3 1/2] " Ilya Leoshkevich
2023-02-24 7:42 ` Philippe Mathieu-Daudé
@ 2023-03-06 21:34 ` Laurent Vivier
2023-03-07 10:32 ` Laurent Vivier
2 siblings, 0 replies; 6+ messages in thread
From: Laurent Vivier @ 2023-03-06 21:34 UTC (permalink / raw)
To: Ilya Leoshkevich, Alex Bennée
Cc: qemu-devel, Christian Borntraeger, Richard Henderson
Le 24/02/2023 à 01:39, Ilya Leoshkevich a écrit :
> target_rlimit64 contains uint64_t fields, so it's 8-byte aligned on
> some hosts, while some guests may align their respective type on a
> 4-byte boundary. This may lead to an unaligned access, which is an UB.
>
> Fix by defining the fields as abi_ullong. This makes the host alignment
> match that of the guest, and lets the compiler know that it should emit
> code that can deal with the guest alignment.
>
> While at it, also use __get_user() and __put_user() instead of
> tswap64().
>
> Fixes: 163a05a8398b ("linux-user: Implement prlimit64 syscall")
> Reported-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
> linux-user/generic/target_resource.h | 4 ++--
> linux-user/syscall.c | 8 ++++----
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/linux-user/generic/target_resource.h b/linux-user/generic/target_resource.h
> index 539d8c46772..37d3eb09b3b 100644
> --- a/linux-user/generic/target_resource.h
> +++ b/linux-user/generic/target_resource.h
> @@ -12,8 +12,8 @@ struct target_rlimit {
> };
>
> struct target_rlimit64 {
> - uint64_t rlim_cur;
> - uint64_t rlim_max;
> + abi_ullong rlim_cur;
> + abi_ullong rlim_max;
> };
>
> #define TARGET_RLIM_INFINITY ((abi_ulong)-1)
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index a6c426d73cf..73082531ffc 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -12886,8 +12886,8 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
> if (!lock_user_struct(VERIFY_READ, target_rnew, arg3, 1)) {
> return -TARGET_EFAULT;
> }
> - rnew.rlim_cur = tswap64(target_rnew->rlim_cur);
> - rnew.rlim_max = tswap64(target_rnew->rlim_max);
> + __get_user(rnew.rlim_cur, &target_rnew->rlim_cur);
> + __get_user(rnew.rlim_max, &target_rnew->rlim_max);
> unlock_user_struct(target_rnew, arg3, 0);
> rnewp = &rnew;
> }
> @@ -12897,8 +12897,8 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
> if (!lock_user_struct(VERIFY_WRITE, target_rold, arg4, 1)) {
> return -TARGET_EFAULT;
> }
> - target_rold->rlim_cur = tswap64(rold.rlim_cur);
> - target_rold->rlim_max = tswap64(rold.rlim_max);
> + __put_user(rold.rlim_cur, &target_rold->rlim_cur);
> + __put_user(rold.rlim_max, &target_rold->rlim_max);
> unlock_user_struct(target_rold, arg4, 1);
> }
> return ret;
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] linux-user: Fix unaligned memory access in prlimit64 syscall
2023-02-24 0:39 ` [PATCH v3 1/2] " Ilya Leoshkevich
2023-02-24 7:42 ` Philippe Mathieu-Daudé
2023-03-06 21:34 ` Laurent Vivier
@ 2023-03-07 10:32 ` Laurent Vivier
2 siblings, 0 replies; 6+ messages in thread
From: Laurent Vivier @ 2023-03-07 10:32 UTC (permalink / raw)
To: Ilya Leoshkevich, Alex Bennée
Cc: qemu-devel, Christian Borntraeger, Richard Henderson
Le 24/02/2023 à 01:39, Ilya Leoshkevich a écrit :
> target_rlimit64 contains uint64_t fields, so it's 8-byte aligned on
> some hosts, while some guests may align their respective type on a
> 4-byte boundary. This may lead to an unaligned access, which is an UB.
>
> Fix by defining the fields as abi_ullong. This makes the host alignment
> match that of the guest, and lets the compiler know that it should emit
> code that can deal with the guest alignment.
>
> While at it, also use __get_user() and __put_user() instead of
> tswap64().
>
> Fixes: 163a05a8398b ("linux-user: Implement prlimit64 syscall")
> Reported-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
> linux-user/generic/target_resource.h | 4 ++--
> linux-user/syscall.c | 8 ++++----
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/linux-user/generic/target_resource.h b/linux-user/generic/target_resource.h
> index 539d8c46772..37d3eb09b3b 100644
> --- a/linux-user/generic/target_resource.h
> +++ b/linux-user/generic/target_resource.h
> @@ -12,8 +12,8 @@ struct target_rlimit {
> };
>
> struct target_rlimit64 {
> - uint64_t rlim_cur;
> - uint64_t rlim_max;
> + abi_ullong rlim_cur;
> + abi_ullong rlim_max;
> };
>
> #define TARGET_RLIM_INFINITY ((abi_ulong)-1)
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index a6c426d73cf..73082531ffc 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -12886,8 +12886,8 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
> if (!lock_user_struct(VERIFY_READ, target_rnew, arg3, 1)) {
> return -TARGET_EFAULT;
> }
> - rnew.rlim_cur = tswap64(target_rnew->rlim_cur);
> - rnew.rlim_max = tswap64(target_rnew->rlim_max);
> + __get_user(rnew.rlim_cur, &target_rnew->rlim_cur);
> + __get_user(rnew.rlim_max, &target_rnew->rlim_max);
> unlock_user_struct(target_rnew, arg3, 0);
> rnewp = &rnew;
> }
> @@ -12897,8 +12897,8 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
> if (!lock_user_struct(VERIFY_WRITE, target_rold, arg4, 1)) {
> return -TARGET_EFAULT;
> }
> - target_rold->rlim_cur = tswap64(rold.rlim_cur);
> - target_rold->rlim_max = tswap64(rold.rlim_max);
> + __put_user(rold.rlim_cur, &target_rold->rlim_cur);
> + __put_user(rold.rlim_max, &target_rold->rlim_max);
> unlock_user_struct(target_rold, arg4, 1);
> }
> return ret;
Applied to my linux-user-for-8.0 branch.
Thanks,
Laurent
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-03-07 10:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-24 0:39 [PATCH v3 0/2] linux-user: Fix unaligned memory access in prlimit64 syscall Ilya Leoshkevich
2023-02-24 0:39 ` [PATCH v3 1/2] " Ilya Leoshkevich
2023-02-24 7:42 ` Philippe Mathieu-Daudé
2023-03-06 21:34 ` Laurent Vivier
2023-03-07 10:32 ` Laurent Vivier
2023-02-24 0:39 ` [PATCH v3 2/2] tests/tcg/linux-test: Add linux-fork-trap test Ilya Leoshkevich
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.