add new classifier result

1 files changed, 366 insertions, 0 deletions

diff --git a/results/classifier/003/other/02364653 b/results/classifier/003/other/02364653
new file mode 100644
index 00000000..2ea6f8bc
--- /dev/null
+++ b/results/classifier/003/other/02364653
@@ -0,0 +1,366 @@
+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
+