diff options
Diffstat (limited to 'results/classifier/014/all/02364653')
| -rw-r--r-- | results/classifier/014/all/02364653 | 390 |
1 files changed, 390 insertions, 0 deletions
diff --git a/results/classifier/014/all/02364653 b/results/classifier/014/all/02364653 new file mode 100644 index 00000000..6b9e2cf9 --- /dev/null +++ b/results/classifier/014/all/02364653 @@ -0,0 +1,390 @@ +graphic: 0.948 +virtual: 0.947 +permissions: 0.944 +kernel: 0.943 +semantic: 0.942 +arm: 0.942 +debug: 0.940 +PID: 0.938 +assembly: 0.936 +alpha: 0.935 +risc-v: 0.934 +performance: 0.934 +user-level: 0.930 +device: 0.928 +register: 0.927 +architecture: 0.927 +boot: 0.925 +hypervisor: 0.925 +socket: 0.924 +vnc: 0.922 +i386: 0.918 +peripherals: 0.913 +mistranslation: 0.912 +KVM: 0.911 +x86: 0.908 +files: 0.908 +operating system: 0.904 +TCG: 0.901 +VMM: 0.897 +ppc: 0.895 +network: 0.881 + +[Qemu-devel] [BUG] Inappropriate size of target_sigset_t + +Hello, Peter, Laurent, + +While working on another problem yesterday, I think I discovered a +long-standing bug in QEMU Linux user mode: our target_sigset_t structure is +eight times smaller as it should be! + +In this code segment from syscalls_def.h: + +#ifdef TARGET_MIPS +#define TARGET_NSIG 128 +#else +#define TARGET_NSIG 64 +#endif +#define TARGET_NSIG_BPW TARGET_ABI_BITS +#define TARGET_NSIG_WORDS (TARGET_NSIG / TARGET_NSIG_BPW) + +typedef struct { + abi_ulong sig[TARGET_NSIG_WORDS]; +} target_sigset_t; + +... TARGET_ABI_BITS should be replaced by eight times smaller constant (in +fact, semantically, we need TARGET_ABI_BYTES, but it is not defined) (what is +needed is actually "a byte per signal" in target_sigset_t, and we allow "a bit +per signal"). + +All this probably sounds to you like something impossible, since this code is +in QEMU "since forever", but I checked everything, and the bug seems real. I +wish you can prove me wrong. + +I just wanted to let you know about this, given the sensitive timing of current +softfreeze, and the fact that I won't be able to do more investigation on this +in coming weeks, since I am busy with other tasks, but perhaps you can analyze +and do something which you consider appropriate. + +Yours, +Aleksandar + +Le 03/07/2019 à 21:46, Aleksandar Markovic a écrit : +> +Hello, Peter, Laurent, +> +> +While working on another problem yesterday, I think I discovered a +> +long-standing bug in QEMU Linux user mode: our target_sigset_t structure is +> +eight times smaller as it should be! +> +> +In this code segment from syscalls_def.h: +> +> +#ifdef TARGET_MIPS +> +#define TARGET_NSIG 128 +> +#else +> +#define TARGET_NSIG 64 +> +#endif +> +#define TARGET_NSIG_BPW TARGET_ABI_BITS +> +#define TARGET_NSIG_WORDS (TARGET_NSIG / TARGET_NSIG_BPW) +> +> +typedef struct { +> +abi_ulong sig[TARGET_NSIG_WORDS]; +> +} target_sigset_t; +> +> +... TARGET_ABI_BITS should be replaced by eight times smaller constant (in +> +fact, semantically, we need TARGET_ABI_BYTES, but it is not defined) (what is +> +needed is actually "a byte per signal" in target_sigset_t, and we allow "a +> +bit per signal"). +TARGET_NSIG is divided by TARGET_ABI_BITS which gives you the number of +abi_ulong words we need in target_sigset_t. + +> +All this probably sounds to you like something impossible, since this code is +> +in QEMU "since forever", but I checked everything, and the bug seems real. I +> +wish you can prove me wrong. +> +> +I just wanted to let you know about this, given the sensitive timing of +> +current softfreeze, and the fact that I won't be able to do more +> +investigation on this in coming weeks, since I am busy with other tasks, but +> +perhaps you can analyze and do something which you consider appropriate. +If I compare with kernel, it looks good: + +In Linux: + + arch/mips/include/uapi/asm/signal.h + + #define _NSIG 128 + #define _NSIG_BPW (sizeof(unsigned long) * 8) + #define _NSIG_WORDS (_NSIG / _NSIG_BPW) + + typedef struct { + unsigned long sig[_NSIG_WORDS]; + } sigset_t; + +_NSIG_BPW is 8 * 8 = 64 on MIPS64 or 4 * 8 = 32 on MIPS + +In QEMU: + +TARGET_NSIG_BPW is TARGET_ABI_BITS which is TARGET_LONG_BITS which is +64 on MIPS64 and 32 on MIPS. + +I think there is no problem. + +Thanks, +Laurent + +From: Laurent Vivier <address@hidden> +> +If I compare with kernel, it looks good: +> +... +> +I think there is no problem. +Sure, thanks for such fast response - again, I am glad if you are right. +However, for some reason, glibc (and musl too) define sigset_t differently than +kernel. Please take a look. I am not sure if this is covered fine in our code. + +Yours, +Aleksandar + +> +Thanks, +> +Laurent + +On Wed, 3 Jul 2019 at 21:20, Aleksandar Markovic <address@hidden> wrote: +> +> +From: Laurent Vivier <address@hidden> +> +> If I compare with kernel, it looks good: +> +> ... +> +> I think there is no problem. +> +> +Sure, thanks for such fast response - again, I am glad if you are right. +> +However, for some reason, glibc (and musl too) define sigset_t differently +> +than kernel. Please take a look. I am not sure if this is covered fine in our +> +code. +Yeah, the libc definitions of sigset_t don't match the +kernel ones (this is for obscure historical reasons IIRC). +We're providing implementations of the target +syscall interface, so our target_sigset_t should be the +target kernel's version (and the target libc's version doesn't +matter to us). On the other hand we will be using the +host libc version, I think, so a little caution is required +and it's possible we have some bugs in our code. + +thanks +-- PMM + +> +From: Peter Maydell <address@hidden> +> +> +On Wed, 3 Jul 2019 at 21:20, Aleksandar Markovic <address@hidden> wrote: +> +> +> +> From: Laurent Vivier <address@hidden> +> +> > If I compare with kernel, it looks good: +> +> > ... +> +> > I think there is no problem. +> +> +> +> Sure, thanks for such fast response - again, I am glad if you are right. +> +> However, for some reason, glibc (and musl too) define sigset_t differently +> +> than kernel. Please take a look. I am not sure if this is covered fine in +> +> our code. +> +> +Yeah, the libc definitions of sigset_t don't match the +> +kernel ones (this is for obscure historical reasons IIRC). +> +We're providing implementations of the target +> +syscall interface, so our target_sigset_t should be the +> +target kernel's version (and the target libc's version doesn't +> +matter to us). On the other hand we will be using the +> +host libc version, I think, so a little caution is required +> +and it's possible we have some bugs in our code. +OK, I gather than this is not something that requires our immediate attention +(for 4.1), but we can analyze it later on. + +Thanks for response!! + +Sincerely, +Aleksandar + +> +thanks +> +-- PMM + +Le 03/07/2019 à 22:28, Peter Maydell a écrit : +> +On Wed, 3 Jul 2019 at 21:20, Aleksandar Markovic <address@hidden> wrote: +> +> +> +> From: Laurent Vivier <address@hidden> +> +>> If I compare with kernel, it looks good: +> +>> ... +> +>> I think there is no problem. +> +> +> +> Sure, thanks for such fast response - again, I am glad if you are right. +> +> However, for some reason, glibc (and musl too) define sigset_t differently +> +> than kernel. Please take a look. I am not sure if this is covered fine in +> +> our code. +> +> +Yeah, the libc definitions of sigset_t don't match the +> +kernel ones (this is for obscure historical reasons IIRC). +> +We're providing implementations of the target +> +syscall interface, so our target_sigset_t should be the +> +target kernel's version (and the target libc's version doesn't +> +matter to us). On the other hand we will be using the +> +host libc version, I think, so a little caution is required +> +and it's possible we have some bugs in our code. +It's why we need host_to_target_sigset_internal() and +target_to_host_sigset_internal() that translates bits and bytes between +guest kernel interface and host libc interface. + +void host_to_target_sigset_internal(target_sigset_t *d, + const sigset_t *s) +{ + int i; + target_sigemptyset(d); + for (i = 1; i <= TARGET_NSIG; i++) { + if (sigismember(s, i)) { + target_sigaddset(d, host_to_target_signal(i)); + } + } +} + +void target_to_host_sigset_internal(sigset_t *d, + const target_sigset_t *s) +{ + int i; + sigemptyset(d); + for (i = 1; i <= TARGET_NSIG; i++) { + if (target_sigismember(s, i)) { + sigaddset(d, target_to_host_signal(i)); + } + } +} + +Thanks, +Laurent + +Hi Aleksandar, + +On Wed, Jul 3, 2019 at 12:48 PM Aleksandar Markovic +<address@hidden> wrote: +> +#define TARGET_NSIG_BPW TARGET_ABI_BITS +> +#define TARGET_NSIG_WORDS (TARGET_NSIG / TARGET_NSIG_BPW) +> +> +typedef struct { +> +abi_ulong sig[TARGET_NSIG_WORDS]; +> +} target_sigset_t; +> +> +... TARGET_ABI_BITS should be replaced by eight times smaller constant (in +> +fact, +> +semantically, we need TARGET_ABI_BYTES, but it is not defined) (what is needed +> +is actually "a byte per signal" in target_sigset_t, and we allow "a bit per +> +signal"). +Why do we need a byte per target signal, if the functions in linux-user/signal.c +operate with bits? + +-- +Thanks. +-- Max + +> +Why do we need a byte per target signal, if the functions in +> +linux-user/signal.c +> +operate with bits? +Max, + +I did not base my findings on code analysis, but on dumping size/offsets of +elements of some structures, as they are emulated in QEMU, and in real systems. +So, I can't really answer your question. + +Yours, +Aleksandar + +> +-- +> +Thanks. +> +-- Max + |