diff options
Diffstat (limited to 'classification_output/03/other/02364653')
| -rw-r--r-- | classification_output/03/other/02364653 | 366 |
1 files changed, 0 insertions, 366 deletions
diff --git a/classification_output/03/other/02364653 b/classification_output/03/other/02364653 deleted file mode 100644 index 2ea6f8bc5..000000000 --- a/classification_output/03/other/02364653 +++ /dev/null @@ -1,366 +0,0 @@ -other: 0.956 -semantic: 0.942 -instruction: 0.927 -boot: 0.925 -mistranslation: 0.912 -KVM: 0.911 -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 - |