* [PULL 1/9] linux-user: Emulate /proc/cpuinfo output for riscv
2023-05-12 11:22 [PULL 0/9] Linux user for 8.1 patches Laurent Vivier
@ 2023-05-12 11:22 ` Laurent Vivier
2023-05-12 11:22 ` [PULL 2/9] linux-user: report ENOTTY for unknown ioctls Laurent Vivier
` (8 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: Laurent Vivier @ 2023-05-12 11:22 UTC (permalink / raw)
To: qemu-devel
Cc: Afonso Bordado, Palmer Dabbelt, Laurent Vivier, Alistair Francis,
LIU Zhiwei
From: Afonso Bordado <afonsobordado@gmail.com>
RISC-V does not expose all extensions via hwcaps, thus some userspace
applications may want to query these via /proc/cpuinfo.
Currently when querying this file the host's file is shown instead
which is slightly confusing. Emulate a basic /proc/cpuinfo file
with mmu info and an ISA string.
Signed-off-by: Afonso Bordado <afonsobordado@gmail.com>
Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>
Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
Message-Id: <167873059442.9885.15152085316575248452-0@git.sr.ht>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
linux-user/syscall.c | 34 +++++++++++++++++++++++++++++--
tests/tcg/riscv64/Makefile.target | 1 +
tests/tcg/riscv64/cpuinfo.c | 30 +++++++++++++++++++++++++++
3 files changed, 63 insertions(+), 2 deletions(-)
create mode 100644 tests/tcg/riscv64/cpuinfo.c
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 69f740ff98c8..085ce530213e 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8231,7 +8231,8 @@ void target_exception_dump(CPUArchState *env, const char *fmt, int code)
}
#if HOST_BIG_ENDIAN != TARGET_BIG_ENDIAN || \
- defined(TARGET_SPARC) || defined(TARGET_M68K) || defined(TARGET_HPPA)
+ defined(TARGET_SPARC) || defined(TARGET_M68K) || defined(TARGET_HPPA) || \
+ defined(TARGET_RISCV)
static int is_proc(const char *filename, const char *entry)
{
return strcmp(filename, entry) == 0;
@@ -8309,6 +8310,35 @@ static int open_cpuinfo(CPUArchState *cpu_env, int fd)
}
#endif
+#if defined(TARGET_RISCV)
+static int open_cpuinfo(CPUArchState *cpu_env, int fd)
+{
+ int i;
+ int num_cpus = sysconf(_SC_NPROCESSORS_ONLN);
+ RISCVCPU *cpu = env_archcpu(cpu_env);
+ const RISCVCPUConfig *cfg = riscv_cpu_cfg((CPURISCVState *) cpu_env);
+ char *isa_string = riscv_isa_string(cpu);
+ const char *mmu;
+
+ if (cfg->mmu) {
+ mmu = (cpu_env->xl == MXL_RV32) ? "sv32" : "sv48";
+ } else {
+ mmu = "none";
+ }
+
+ for (i = 0; i < num_cpus; i++) {
+ dprintf(fd, "processor\t: %d\n", i);
+ dprintf(fd, "hart\t\t: %d\n", i);
+ dprintf(fd, "isa\t\t: %s\n", isa_string);
+ dprintf(fd, "mmu\t\t: %s\n", mmu);
+ dprintf(fd, "uarch\t\t: qemu\n\n");
+ }
+
+ g_free(isa_string);
+ return 0;
+}
+#endif
+
#if defined(TARGET_M68K)
static int open_hardware(CPUArchState *cpu_env, int fd)
{
@@ -8333,7 +8363,7 @@ static int do_openat(CPUArchState *cpu_env, int dirfd, const char *pathname, int
#if HOST_BIG_ENDIAN != TARGET_BIG_ENDIAN
{ "/proc/net/route", open_net_route, is_proc },
#endif
-#if defined(TARGET_SPARC) || defined(TARGET_HPPA)
+#if defined(TARGET_SPARC) || defined(TARGET_HPPA) || defined(TARGET_RISCV)
{ "/proc/cpuinfo", open_cpuinfo, is_proc },
#endif
#if defined(TARGET_M68K)
diff --git a/tests/tcg/riscv64/Makefile.target b/tests/tcg/riscv64/Makefile.target
index 9973ba3b5f8c..2da65dccaf6a 100644
--- a/tests/tcg/riscv64/Makefile.target
+++ b/tests/tcg/riscv64/Makefile.target
@@ -4,6 +4,7 @@
VPATH += $(SRC_PATH)/tests/tcg/riscv64
TESTS += test-div
TESTS += noexec
+TESTS += cpuinfo
# Disable compressed instructions for test-noc
TESTS += test-noc
diff --git a/tests/tcg/riscv64/cpuinfo.c b/tests/tcg/riscv64/cpuinfo.c
new file mode 100644
index 000000000000..296abd0a8cf9
--- /dev/null
+++ b/tests/tcg/riscv64/cpuinfo.c
@@ -0,0 +1,30 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <assert.h>
+
+#define BUFFER_SIZE 1024
+
+int main(void)
+{
+ char buffer[BUFFER_SIZE];
+ FILE *fp = fopen("/proc/cpuinfo", "r");
+ assert(fp != NULL);
+
+ while (fgets(buffer, BUFFER_SIZE, fp) != NULL) {
+ if (strstr(buffer, "processor") != NULL) {
+ assert(strstr(buffer, "processor\t: ") == buffer);
+ } else if (strstr(buffer, "hart") != NULL) {
+ assert(strstr(buffer, "hart\t\t: ") == buffer);
+ } else if (strstr(buffer, "isa") != NULL) {
+ assert(strcmp(buffer, "isa\t\t: rv64imafdc_zicsr_zifencei\n") == 0);
+ } else if (strstr(buffer, "mmu") != NULL) {
+ assert(strcmp(buffer, "mmu\t\t: sv48\n") == 0);
+ } else if (strstr(buffer, "uarch") != NULL) {
+ assert(strcmp(buffer, "uarch\t\t: qemu\n") == 0);
+ }
+ }
+
+ fclose(fp);
+ return 0;
+}
--
2.40.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PULL 2/9] linux-user: report ENOTTY for unknown ioctls
2023-05-12 11:22 [PULL 0/9] Linux user for 8.1 patches Laurent Vivier
2023-05-12 11:22 ` [PULL 1/9] linux-user: Emulate /proc/cpuinfo output for riscv Laurent Vivier
@ 2023-05-12 11:22 ` Laurent Vivier
2023-05-12 11:22 ` [PULL 3/9] linux-user: Add move_mount() syscall Laurent Vivier
` (7 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: Laurent Vivier @ 2023-05-12 11:22 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Weißschuh, Philippe Mathieu-Daudé,
Laurent Vivier
From: Thomas Weißschuh <thomas@t-8ch.de>
The correct error number for unknown ioctls is ENOTTY.
ENOSYS would mean that the ioctl() syscall itself is not implemented,
which is very improbable and unexpected for userspace.
ENOTTY means "Inappropriate ioctl for device". This is what the kernel
returns on unknown ioctls, what qemu is trying to express and what
userspace is prepared to handle.
Signed-off-by: Thomas Weißschuh <thomas@t-8ch.de>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20230426070659.80649-1-thomas@t-8ch.de>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
linux-user/syscall.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 085ce530213e..954ed14df4c0 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -5747,7 +5747,7 @@ static abi_long do_ioctl(int fd, int cmd, abi_long arg)
if (ie->target_cmd == 0) {
qemu_log_mask(
LOG_UNIMP, "Unsupported ioctl: cmd=0x%04lx\n", (long)cmd);
- return -TARGET_ENOSYS;
+ return -TARGET_ENOTTY;
}
if (ie->target_cmd == cmd)
break;
@@ -5759,7 +5759,7 @@ static abi_long do_ioctl(int fd, int cmd, abi_long arg)
} else if (!ie->host_cmd) {
/* Some architectures define BSD ioctls in their headers
that are not implemented in Linux. */
- return -TARGET_ENOSYS;
+ return -TARGET_ENOTTY;
}
switch(arg_type[0]) {
@@ -5817,7 +5817,7 @@ static abi_long do_ioctl(int fd, int cmd, abi_long arg)
qemu_log_mask(LOG_UNIMP,
"Unsupported ioctl type: cmd=0x%04lx type=%d\n",
(long)cmd, arg_type[0]);
- ret = -TARGET_ENOSYS;
+ ret = -TARGET_ENOTTY;
break;
}
return ret;
--
2.40.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PULL 3/9] linux-user: Add move_mount() syscall
2023-05-12 11:22 [PULL 0/9] Linux user for 8.1 patches Laurent Vivier
2023-05-12 11:22 ` [PULL 1/9] linux-user: Emulate /proc/cpuinfo output for riscv Laurent Vivier
2023-05-12 11:22 ` [PULL 2/9] linux-user: report ENOTTY for unknown ioctls Laurent Vivier
@ 2023-05-12 11:22 ` Laurent Vivier
2023-05-12 11:22 ` [PULL 4/9] linux-user: Add open_tree() syscall Laurent Vivier
` (6 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: Laurent Vivier @ 2023-05-12 11:22 UTC (permalink / raw)
To: qemu-devel; +Cc: Thomas Weißschuh, Laurent Vivier
From: Thomas Weißschuh <thomas@t-8ch.de>
Signed-off-by: Thomas Weißschuh <thomas@t-8ch.de>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20230424153429.276788-1-thomas@t-8ch.de>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
linux-user/syscall.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 954ed14df4c0..fccc46e609c6 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -9169,6 +9169,33 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
unlock_user(p, arg1, 0);
return ret;
#endif
+#ifdef TARGET_NR_move_mount
+ case TARGET_NR_move_mount:
+ {
+ void *p2, *p4;
+
+ if (!arg2 || !arg4) {
+ return -TARGET_EFAULT;
+ }
+
+ p2 = lock_user_string(arg2);
+ if (!p2) {
+ return -TARGET_EFAULT;
+ }
+
+ p4 = lock_user_string(arg4);
+ if (!p4) {
+ unlock_user(p2, arg2, 0);
+ return -TARGET_EFAULT;
+ }
+ ret = get_errno(move_mount(arg1, p2, arg3, p4, arg5));
+
+ unlock_user(p2, arg2, 0);
+ unlock_user(p4, arg4, 0);
+
+ return ret;
+ }
+#endif
#ifdef TARGET_NR_stime /* not on alpha */
case TARGET_NR_stime:
{
--
2.40.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PULL 4/9] linux-user: Add open_tree() syscall
2023-05-12 11:22 [PULL 0/9] Linux user for 8.1 patches Laurent Vivier
` (2 preceding siblings ...)
2023-05-12 11:22 ` [PULL 3/9] linux-user: Add move_mount() syscall Laurent Vivier
@ 2023-05-12 11:22 ` Laurent Vivier
2023-05-12 11:22 ` [PULL 5/9] linux-user/main: Use list_cpus() instead of cpu_list() Laurent Vivier
` (5 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: Laurent Vivier @ 2023-05-12 11:22 UTC (permalink / raw)
To: qemu-devel; +Cc: Thomas Weißschuh, Laurent Vivier
From: Thomas Weißschuh <thomas@t-8ch.de>
Signed-off-by: Thomas Weißschuh <thomas@t-8ch.de>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20230424153429.276788-2-thomas@t-8ch.de>
[lv: move declaration at the beginning of the block]
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
linux-user/syscall.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index fccc46e609c6..95da74ab1234 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -9196,6 +9196,33 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
return ret;
}
#endif
+#ifdef TARGET_NR_open_tree
+ case TARGET_NR_open_tree:
+ {
+ void *p2;
+ int host_flags;
+
+ if (!arg2) {
+ return -TARGET_EFAULT;
+ }
+
+ p2 = lock_user_string(arg2);
+ if (!p2) {
+ return -TARGET_EFAULT;
+ }
+
+ host_flags = arg3 & ~TARGET_O_CLOEXEC;
+ if (arg3 & TARGET_O_CLOEXEC) {
+ host_flags |= O_CLOEXEC;
+ }
+
+ ret = get_errno(open_tree(arg1, p2, host_flags));
+
+ unlock_user(p2, arg2, 0);
+
+ return ret;
+ }
+#endif
#ifdef TARGET_NR_stime /* not on alpha */
case TARGET_NR_stime:
{
--
2.40.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PULL 5/9] linux-user/main: Use list_cpus() instead of cpu_list()
2023-05-12 11:22 [PULL 0/9] Linux user for 8.1 patches Laurent Vivier
` (3 preceding siblings ...)
2023-05-12 11:22 ` [PULL 4/9] linux-user: Add open_tree() syscall Laurent Vivier
@ 2023-05-12 11:22 ` Laurent Vivier
2023-05-12 11:22 ` [PULL 6/9] linux-user: Add new flag VERIFY_NONE Laurent Vivier
` (4 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: Laurent Vivier @ 2023-05-12 11:22 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Richard Henderson, Philippe Mathieu-Daudé,
Laurent Vivier
From: Thomas Huth <thuth@redhat.com>
This way we can get rid of the if'deffery and the XXX comment
here (it's repeated in the list_cpus() function anyway).
Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20230424122126.236586-1-thuth@redhat.com>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
linux-user/main.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/linux-user/main.c b/linux-user/main.c
index fe03293516a5..aece4d9e9119 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -359,10 +359,7 @@ static void handle_arg_cpu(const char *arg)
{
cpu_model = strdup(arg);
if (cpu_model == NULL || is_help_option(cpu_model)) {
- /* XXX: implement xxx_cpu_list for targets that still miss it */
-#if defined(cpu_list)
- cpu_list();
-#endif
+ list_cpus();
exit(EXIT_FAILURE);
}
}
--
2.40.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PULL 6/9] linux-user: Add new flag VERIFY_NONE
2023-05-12 11:22 [PULL 0/9] Linux user for 8.1 patches Laurent Vivier
` (4 preceding siblings ...)
2023-05-12 11:22 ` [PULL 5/9] linux-user/main: Use list_cpus() instead of cpu_list() Laurent Vivier
@ 2023-05-12 11:22 ` Laurent Vivier
2023-05-12 11:22 ` [PULL 7/9] linux-user: Don't require PROT_READ for mincore Laurent Vivier
` (3 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: Laurent Vivier @ 2023-05-12 11:22 UTC (permalink / raw)
To: qemu-devel; +Cc: Thomas Weißschuh, Laurent Vivier
From: Thomas Weißschuh <thomas@t-8ch.de>
This can be used to validate that an address range is mapped but without
being readable or writable.
It will be used by an updated implementation of mincore().
Signed-off-by: Thomas Weißschuh <thomas@t-8ch.de>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20230422100314.1650-2-thomas@t-8ch.de>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
linux-user/qemu.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index e2e93fbd1d5d..92f9f5af41c7 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -168,6 +168,7 @@ abi_long do_brk(abi_ulong new_brk);
/* user access */
+#define VERIFY_NONE 0
#define VERIFY_READ PAGE_READ
#define VERIFY_WRITE (PAGE_READ | PAGE_WRITE)
--
2.40.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PULL 7/9] linux-user: Don't require PROT_READ for mincore
2023-05-12 11:22 [PULL 0/9] Linux user for 8.1 patches Laurent Vivier
` (5 preceding siblings ...)
2023-05-12 11:22 ` [PULL 6/9] linux-user: Add new flag VERIFY_NONE Laurent Vivier
@ 2023-05-12 11:22 ` Laurent Vivier
2023-05-12 11:22 ` [PULL 8/9] linux-user: Fix mips fp64 executables loading Laurent Vivier
` (2 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: Laurent Vivier @ 2023-05-12 11:22 UTC (permalink / raw)
To: qemu-devel; +Cc: Thomas Weißschuh, Laurent Vivier
From: Thomas Weißschuh <thomas@t-8ch.de>
The kernel does not require PROT_READ for addresses passed to mincore.
For example the fincore(1) tool from util-linux uses PROT_NONE and
currently does not work under qemu-user.
Example (with fincore(1) from util-linux 2.38):
$ fincore /proc/self/exe
RES PAGES SIZE FILE
24K 6 22.1K /proc/self/exe
$ qemu-x86_64 /usr/bin/fincore /proc/self/exe
fincore: failed to do mincore: /proc/self/exe: Cannot allocate memory
With this patch:
$ ./build/qemu-x86_64 /usr/bin/fincore /proc/self/exe
RES PAGES SIZE FILE
24K 6 22.1K /proc/self/exe
Signed-off-by: Thomas Weißschuh <thomas@t-8ch.de>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20230422100314.1650-3-thomas@t-8ch.de>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
linux-user/syscall.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 95da74ab1234..7170332041ca 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -11981,7 +11981,7 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
#ifdef TARGET_NR_mincore
case TARGET_NR_mincore:
{
- void *a = lock_user(VERIFY_READ, arg1, arg2, 0);
+ void *a = lock_user(VERIFY_NONE, arg1, arg2, 0);
if (!a) {
return -TARGET_ENOMEM;
}
--
2.40.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PULL 8/9] linux-user: Fix mips fp64 executables loading
2023-05-12 11:22 [PULL 0/9] Linux user for 8.1 patches Laurent Vivier
` (6 preceding siblings ...)
2023-05-12 11:22 ` [PULL 7/9] linux-user: Don't require PROT_READ for mincore Laurent Vivier
@ 2023-05-12 11:22 ` Laurent Vivier
2023-05-12 11:22 ` [PULL 9/9] linux-user: fix getgroups/setgroups allocations Laurent Vivier
2023-05-13 6:36 ` [PULL 0/9] Linux user for 8.1 patches Richard Henderson
9 siblings, 0 replies; 14+ messages in thread
From: Laurent Vivier @ 2023-05-12 11:22 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniil Kovalev, Jiaxun Yang, Laurent Vivier
From: Daniil Kovalev <dkovalev@compiler-toolchain-for.me>
If a program requires fr1, we should set the FR bit of CP0 control status
register and add F64 hardware flag. The corresponding `else if` branch
statement is copied from the linux kernel sources (see `arch_check_elf` function
in linux/arch/mips/kernel/elf.c).
Signed-off-by: Daniil Kovalev <dkovalev@compiler-toolchain-for.me>
Reviewed-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
Message-Id: <20230404052153.16617-1-dkovalev@compiler-toolchain-for.me>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
linux-user/mips/cpu_loop.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/linux-user/mips/cpu_loop.c b/linux-user/mips/cpu_loop.c
index d5c1c7941d34..8735e58bada0 100644
--- a/linux-user/mips/cpu_loop.c
+++ b/linux-user/mips/cpu_loop.c
@@ -290,7 +290,10 @@ void target_cpu_copy_regs(CPUArchState *env, struct target_pt_regs *regs)
env->CP0_Status |= (1 << CP0St_FR);
env->hflags |= MIPS_HFLAG_F64;
}
- } else if (!prog_req.fre && !prog_req.frdefault &&
+ } else if (prog_req.fr1) {
+ env->CP0_Status |= (1 << CP0St_FR);
+ env->hflags |= MIPS_HFLAG_F64;
+ } else if (!prog_req.fre && !prog_req.frdefault &&
!prog_req.fr1 && !prog_req.single && !prog_req.soft) {
fprintf(stderr, "qemu: Can't find a matching FPU mode\n");
exit(1);
--
2.40.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PULL 9/9] linux-user: fix getgroups/setgroups allocations
2023-05-12 11:22 [PULL 0/9] Linux user for 8.1 patches Laurent Vivier
` (7 preceding siblings ...)
2023-05-12 11:22 ` [PULL 8/9] linux-user: Fix mips fp64 executables loading Laurent Vivier
@ 2023-05-12 11:22 ` Laurent Vivier
2023-05-13 6:36 ` [PULL 0/9] Linux user for 8.1 patches Richard Henderson
9 siblings, 0 replies; 14+ messages in thread
From: Laurent Vivier @ 2023-05-12 11:22 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Tokarev, Laurent Vivier
From: Michael Tokarev <mjt@tls.msk.ru>
linux-user getgroups(), setgroups(), getgroups32() and setgroups32()
used alloca() to allocate grouplist arrays, with unchecked gidsetsize
coming from the "guest". With NGROUPS_MAX being 65536 (linux, and it
is common for an application to allocate NGROUPS_MAX for getgroups()),
this means a typical allocation is half the megabyte on the stack.
Which just overflows stack, which leads to immediate SIGSEGV in actual
system getgroups() implementation.
An example of such issue is aptitude, eg
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=811087#72
Cap gidsetsize to NGROUPS_MAX (return EINVAL if it is larger than that),
and use heap allocation for grouplist instead of alloca(). While at it,
fix coding style and make all 4 implementations identical.
Try to not impose random limits - for example, allow gidsetsize to be
negative for getgroups() - just do not allocate negative-sized grouplist
in this case but still do actual getgroups() call. But do not allow
negative gidsetsize for setgroups() since its argument is unsigned.
Capping by NGROUPS_MAX seems a bit arbitrary, - we can do more, it is
not an error if set size will be NGROUPS_MAX+1. But we should not allow
integer overflow for the array being allocated. Maybe it is enough to
just call g_try_new() and return ENOMEM if it fails.
Maybe there's also no need to convert setgroups() since this one is
usually smaller and known beforehand (KERN_NGROUPS_MAX is actually 63, -
this is apparently a kernel-imposed limit for runtime group set).
The patch fixes aptitude segfault mentioned above.
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
Message-Id: <20230409105327.1273372-1-mjt@msgid.tls.msk.ru>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
linux-user/syscall.c | 99 ++++++++++++++++++++++++++++++--------------
1 file changed, 68 insertions(+), 31 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 7170332041ca..f49fbd0529d9 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -11559,39 +11559,58 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
{
int gidsetsize = arg1;
target_id *target_grouplist;
- gid_t *grouplist;
+ g_autofree gid_t *grouplist = NULL;
int i;
- grouplist = alloca(gidsetsize * sizeof(gid_t));
+ if (gidsetsize > NGROUPS_MAX) {
+ return -TARGET_EINVAL;
+ }
+ if (gidsetsize > 0) {
+ grouplist = g_try_new(gid_t, gidsetsize);
+ if (!grouplist) {
+ return -TARGET_ENOMEM;
+ }
+ }
ret = get_errno(getgroups(gidsetsize, grouplist));
- if (gidsetsize == 0)
- return ret;
- if (!is_error(ret)) {
- target_grouplist = lock_user(VERIFY_WRITE, arg2, gidsetsize * sizeof(target_id), 0);
- if (!target_grouplist)
+ if (!is_error(ret) && gidsetsize > 0) {
+ target_grouplist = lock_user(VERIFY_WRITE, arg2,
+ gidsetsize * sizeof(target_id), 0);
+ if (!target_grouplist) {
return -TARGET_EFAULT;
- for(i = 0;i < ret; i++)
+ }
+ for (i = 0; i < ret; i++) {
target_grouplist[i] = tswapid(high2lowgid(grouplist[i]));
- unlock_user(target_grouplist, arg2, gidsetsize * sizeof(target_id));
+ }
+ unlock_user(target_grouplist, arg2,
+ gidsetsize * sizeof(target_id));
}
+ return ret;
}
- return ret;
case TARGET_NR_setgroups:
{
int gidsetsize = arg1;
target_id *target_grouplist;
- gid_t *grouplist = NULL;
+ g_autofree gid_t *grouplist = NULL;
int i;
- if (gidsetsize) {
- grouplist = alloca(gidsetsize * sizeof(gid_t));
- target_grouplist = lock_user(VERIFY_READ, arg2, gidsetsize * sizeof(target_id), 1);
+
+ if (gidsetsize > NGROUPS_MAX || gidsetsize < 0) {
+ return -TARGET_EINVAL;
+ }
+ if (gidsetsize > 0) {
+ grouplist = g_try_new(gid_t, gidsetsize);
+ if (!grouplist) {
+ return -TARGET_ENOMEM;
+ }
+ target_grouplist = lock_user(VERIFY_READ, arg2,
+ gidsetsize * sizeof(target_id), 1);
if (!target_grouplist) {
return -TARGET_EFAULT;
}
for (i = 0; i < gidsetsize; i++) {
grouplist[i] = low2highgid(tswapid(target_grouplist[i]));
}
- unlock_user(target_grouplist, arg2, 0);
+ unlock_user(target_grouplist, arg2,
+ gidsetsize * sizeof(target_id));
}
return get_errno(setgroups(gidsetsize, grouplist));
}
@@ -11876,41 +11895,59 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,
{
int gidsetsize = arg1;
uint32_t *target_grouplist;
- gid_t *grouplist;
+ g_autofree gid_t *grouplist = NULL;
int i;
- grouplist = alloca(gidsetsize * sizeof(gid_t));
+ if (gidsetsize > NGROUPS_MAX) {
+ return -TARGET_EINVAL;
+ }
+ if (gidsetsize > 0) {
+ grouplist = g_try_new(gid_t, gidsetsize);
+ if (!grouplist) {
+ return -TARGET_ENOMEM;
+ }
+ }
ret = get_errno(getgroups(gidsetsize, grouplist));
- if (gidsetsize == 0)
- return ret;
- if (!is_error(ret)) {
- target_grouplist = lock_user(VERIFY_WRITE, arg2, gidsetsize * 4, 0);
+ if (!is_error(ret) && gidsetsize > 0) {
+ target_grouplist = lock_user(VERIFY_WRITE, arg2,
+ gidsetsize * 4, 0);
if (!target_grouplist) {
return -TARGET_EFAULT;
}
- for(i = 0;i < ret; i++)
+ for (i = 0; i < ret; i++) {
target_grouplist[i] = tswap32(grouplist[i]);
+ }
unlock_user(target_grouplist, arg2, gidsetsize * 4);
}
+ return ret;
}
- return ret;
#endif
#ifdef TARGET_NR_setgroups32
case TARGET_NR_setgroups32:
{
int gidsetsize = arg1;
uint32_t *target_grouplist;
- gid_t *grouplist;
+ g_autofree gid_t *grouplist = NULL;
int i;
- grouplist = alloca(gidsetsize * sizeof(gid_t));
- target_grouplist = lock_user(VERIFY_READ, arg2, gidsetsize * 4, 1);
- if (!target_grouplist) {
- return -TARGET_EFAULT;
+ if (gidsetsize > NGROUPS_MAX || gidsetsize < 0) {
+ return -TARGET_EINVAL;
+ }
+ if (gidsetsize > 0) {
+ grouplist = g_try_new(gid_t, gidsetsize);
+ if (!grouplist) {
+ return -TARGET_ENOMEM;
+ }
+ target_grouplist = lock_user(VERIFY_READ, arg2,
+ gidsetsize * 4, 1);
+ if (!target_grouplist) {
+ return -TARGET_EFAULT;
+ }
+ for (i = 0; i < gidsetsize; i++) {
+ grouplist[i] = tswap32(target_grouplist[i]);
+ }
+ unlock_user(target_grouplist, arg2, 0);
}
- for(i = 0;i < gidsetsize; i++)
- grouplist[i] = tswap32(target_grouplist[i]);
- unlock_user(target_grouplist, arg2, 0);
return get_errno(setgroups(gidsetsize, grouplist));
}
#endif
--
2.40.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PULL 0/9] Linux user for 8.1 patches
2023-05-12 11:22 [PULL 0/9] Linux user for 8.1 patches Laurent Vivier
` (8 preceding siblings ...)
2023-05-12 11:22 ` [PULL 9/9] linux-user: fix getgroups/setgroups allocations Laurent Vivier
@ 2023-05-13 6:36 ` Richard Henderson
9 siblings, 0 replies; 14+ messages in thread
From: Richard Henderson @ 2023-05-13 6:36 UTC (permalink / raw)
To: Laurent Vivier, qemu-devel
On 5/12/23 12:22, Laurent Vivier wrote:
> The following changes since commit 7c18f2d663521f1b31b821a13358ce38075eaf7d:
>
> Merge tag 'for-upstream' ofhttps://gitlab.com/bonzini/qemu into staging (2023-04-29 23:07:17 +0100)
>
> are available in the Git repository at:
>
> https://github.com/vivier/qemu.git tags/linux-user-for-8.1-pull-request
>
> for you to fetch changes up to 3e7f6c76d529dd4096d043cbd16593e40b469e70:
>
> linux-user: fix getgroups/setgroups allocations (2023-05-05 14:37:05 +0200)
>
> ----------------------------------------------------------------
> linux-user pull request 20230512
>
> add open_tree(), move_mount()
> add /proc/cpuinfo for riscv
> fixes and cleanup
Has CI failures:
https://gitlab.com/qemu-project/qemu/-/jobs/4277490874#L884
9191 | ret = get_errno(move_mount(arg1, p2, arg3, p4, arg5));
| ^~~~~~~~~~
../linux-user/syscall.c:9191:29: error: nested extern declaration of 'move_mount'
[-Werror=nested-externs]
../linux-user/syscall.c:9219:29: error: implicit declaration of function 'open_tree'
[-Werror=implicit-function-declaration]
9219 | ret = get_errno(open_tree(arg1, p2, host_flags));
| ^~~~~~~~~
r~
^ permalink raw reply [flat|nested] 14+ messages in thread