|
From: Konstantin S. <kon...@gm...> - 2009-10-22 05:56:46
|
Hi,
Here is a small bug which leads to memcheck false positives on x86_64.
In short, sizeof(siginfo_t)==136, while it needs to be 128.
Would you mind fixing this (see patch below)?
Test:
$ cat sigqueue_test.c
#include <signal.h>
#include <syscall.h>
#include <string.h>
#include <stdlib.h>
#include <stdio.h>
int main() {
siginfo_t *si;
const size_t sz = sizeof(*si);
printf("sizeof(*si) = %lu\n", sz);
printf("%ld %ld %ld %ld\n",
(char*)&si->si_signo - (char*)si,
(char*)&si->si_errno - (char*)si,
(char*)&si->si_code - (char*)si,
(char*)&si->_sifields - (char*)si
);
si = malloc(sz);
memset(si, 0, sz);
si->si_signo = SIGWINCH;
si->si_code = SI_QUEUE;
si->si_pid = getpid();
si->si_uid = getuid();
syscall(__NR_rt_sigqueueinfo, getpid(), SIGWINCH, si);
return 0;
}
$ gcc -g sigqueue_test.c && ./a.out && ~/valgrind/trunk/inst/bin/valgrind
./a.out
sizeof(*si) = 128
0 4 8 16
==13294== Memcheck, a memory error detector
==13294== Copyright (C) 2002-2009, and GNU GPL'd, by Julian Seward et al.
==13294== Using Valgrind-3.6.0.SVN and LibVEX; rerun with -h for copyright
info
==13294== Command: ./a.out
==13294==
sizeof(*si) = 128
0 4 8 16
==13294== Syscall param rt_sigqueueinfo(uinfo) points to unaddressable
byte(s)
==13294== at 0x4EE82E9: syscall (in /usr/grte/v1/lib64/libc-2.3.6.so)
==13294== by 0x400750: main (sigqueue_test.c:24)
==13294== Address 0x516a0c0 is 0 bytes after a block of size 128 alloc'd
==13294== at 0x4C1BE27: malloc (vg_replace_malloc.c:195)
==13294== by 0x4006DB: main (sigqueue_test.c:18)
==13294==
==13294==
==13294== HEAP SUMMARY:
==13294== in use at exit: 128 bytes in 1 blocks
==13294== total heap usage: 1 allocs, 0 frees, 128 bytes allocated
==13294==
==13294== LEAK SUMMARY:
==13294== definitely lost: 128 bytes in 1 blocks
==13294== indirectly lost: 0 bytes in 0 blocks
==13294== possibly lost: 0 bytes in 0 blocks
==13294== still reachable: 0 bytes in 0 blocks
==13294== suppressed: 0 bytes in 0 blocks
==13294== Rerun with --leak-check=full to see details of leaked memory
==13294==
==13294== For counts of detected and suppressed errors, rerun with: -v
==13294== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 5 from 5)
Fix (something like):
Index: include/vki/vki-linux.h
===================================================================
--- include/vki/vki-linux.h (revision 10904)
+++ include/vki/vki-linux.h (working copy)
@@ -340,16 +340,17 @@
void __user *sival_ptr;
} vki_sigval_t;
-#ifndef __VKI_ARCH_SI_PREAMBLE_SIZE
-#define __VKI_ARCH_SI_PREAMBLE_SIZE (3 * sizeof(int))
-#endif
-
#define VKI_SI_MAX_SIZE 128
#ifndef VKI_SI_PAD_SIZE
-#define VKI_SI_PAD_SIZE ((VKI_SI_MAX_SIZE -
__VKI_ARCH_SI_PREAMBLE_SIZE) / sizeof(int))
+# if defined (VGA_amd64) // or whatever is right for 64-bit arch.
+# define VKI_SI_PAD_SIZE ((VKI_SI_MAX_SIZE / sizeof (int)) - 4)
+# else
+# define VKI_SI_PAD_SIZE ((VKI_SI_MAX_SIZE / sizeof (int)) - 3)
+# endif
#endif
+
#ifndef __VKI_ARCH_SI_UID_T
#define __VKI_ARCH_SI_UID_T vki_uid_t
#endif
Thanks,
--kcc
|
|
From: Bart V. A. <bar...@gm...> - 2009-10-27 14:32:22
|
On Thu, Oct 22, 2009 at 6:56 AM, Konstantin Serebryany
<kon...@gm...> wrote:
> Hi,
> Here is a small bug which leads to memcheck false positives on x86_64.
> In short, sizeof(siginfo_t)==136, while it needs to be 128.
> Would you mind fixing this (see patch below)?
> Test:
> $ cat sigqueue_test.c
> #include <signal.h>
> #include <syscall.h>
> #include <string.h>
> #include <stdlib.h>
> #include <stdio.h>
> int main() {
> siginfo_t *si;
> const size_t sz = sizeof(*si);
> printf("sizeof(*si) = %lu\n", sz);
> printf("%ld %ld %ld %ld\n",
> (char*)&si->si_signo - (char*)si,
> (char*)&si->si_errno - (char*)si,
> (char*)&si->si_code - (char*)si,
> (char*)&si->_sifields - (char*)si
> );
> si = malloc(sz);
> memset(si, 0, sz);
> si->si_signo = SIGWINCH;
> si->si_code = SI_QUEUE;
> si->si_pid = getpid();
> si->si_uid = getuid();
> syscall(__NR_rt_sigqueueinfo, getpid(), SIGWINCH, si);
> return 0;
> }
> $ gcc -g sigqueue_test.c && ./a.out && ~/valgrind/trunk/inst/bin/valgrind
> ./a.out
> sizeof(*si) = 128
> 0 4 8 16
> ==13294== Memcheck, a memory error detector
> ==13294== Copyright (C) 2002-2009, and GNU GPL'd, by Julian Seward et al.
> ==13294== Using Valgrind-3.6.0.SVN and LibVEX; rerun with -h for copyright
> info
> ==13294== Command: ./a.out
> ==13294==
> sizeof(*si) = 128
> 0 4 8 16
> ==13294== Syscall param rt_sigqueueinfo(uinfo) points to unaddressable
> byte(s)
> ==13294== at 0x4EE82E9: syscall (in /usr/grte/v1/lib64/libc-2.3.6.so)
> ==13294== by 0x400750: main (sigqueue_test.c:24)
> ==13294== Address 0x516a0c0 is 0 bytes after a block of size 128 alloc'd
> ==13294== at 0x4C1BE27: malloc (vg_replace_malloc.c:195)
> ==13294== by 0x4006DB: main (sigqueue_test.c:18)
> ==13294==
> ==13294==
> ==13294== HEAP SUMMARY:
> ==13294== in use at exit: 128 bytes in 1 blocks
> ==13294== total heap usage: 1 allocs, 0 frees, 128 bytes allocated
> ==13294==
> ==13294== LEAK SUMMARY:
> ==13294== definitely lost: 128 bytes in 1 blocks
> ==13294== indirectly lost: 0 bytes in 0 blocks
> ==13294== possibly lost: 0 bytes in 0 blocks
> ==13294== still reachable: 0 bytes in 0 blocks
> ==13294== suppressed: 0 bytes in 0 blocks
> ==13294== Rerun with --leak-check=full to see details of leaked memory
> ==13294==
> ==13294== For counts of detected and suppressed errors, rerun with: -v
> ==13294== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 5 from 5)
>
>
> Fix (something like):
> Index: include/vki/vki-linux.h
> ===================================================================
> --- include/vki/vki-linux.h (revision 10904)
> +++ include/vki/vki-linux.h (working copy)
> @@ -340,16 +340,17 @@
> void __user *sival_ptr;
> } vki_sigval_t;
> -#ifndef __VKI_ARCH_SI_PREAMBLE_SIZE
> -#define __VKI_ARCH_SI_PREAMBLE_SIZE (3 * sizeof(int))
> -#endif
> -
> #define VKI_SI_MAX_SIZE 128
> #ifndef VKI_SI_PAD_SIZE
> -#define VKI_SI_PAD_SIZE ((VKI_SI_MAX_SIZE -
> __VKI_ARCH_SI_PREAMBLE_SIZE) / sizeof(int))
> +# if defined (VGA_amd64) // or whatever is right for 64-bit arch.
> +# define VKI_SI_PAD_SIZE ((VKI_SI_MAX_SIZE / sizeof (int)) - 4)
> +# else
> +# define VKI_SI_PAD_SIZE ((VKI_SI_MAX_SIZE / sizeof (int)) - 3)
> +# endif
> #endif
> +
> #ifndef __VKI_ARCH_SI_UID_T
> #define __VKI_ARCH_SI_UID_T vki_uid_t
> #endif
I have added a modified version of your test program to the repository
and have filed this issue in bugzilla
(https://bugs.kde.org/show_bug.cgi?id=212064). But I'm not sure that
the proposed fix makes sense. The proposed modifications do not match
the definitions in the kernel header files (<asm-generic/siginfo.h>).
Bart.
|
|
From: Konstantin S. <kon...@gm...> - 2009-10-27 15:22:48
|
On Tue, Oct 27, 2009 at 5:32 PM, Bart Van Assche
<bar...@gm...>wrote:
> On Thu, Oct 22, 2009 at 6:56 AM, Konstantin Serebryany
> <kon...@gm...> wrote:
> > Hi,
> > Here is a small bug which leads to memcheck false positives on x86_64.
> > In short, sizeof(siginfo_t)==136, while it needs to be 128.
> > Would you mind fixing this (see patch below)?
> > Test:
> > $ cat sigqueue_test.c
> > #include <signal.h>
> > #include <syscall.h>
> > #include <string.h>
> > #include <stdlib.h>
> > #include <stdio.h>
> > int main() {
> > siginfo_t *si;
> > const size_t sz = sizeof(*si);
> > printf("sizeof(*si) = %lu\n", sz);
> > printf("%ld %ld %ld %ld\n",
> > (char*)&si->si_signo - (char*)si,
> > (char*)&si->si_errno - (char*)si,
> > (char*)&si->si_code - (char*)si,
> > (char*)&si->_sifields - (char*)si
> > );
> > si = malloc(sz);
> > memset(si, 0, sz);
> > si->si_signo = SIGWINCH;
> > si->si_code = SI_QUEUE;
> > si->si_pid = getpid();
> > si->si_uid = getuid();
> > syscall(__NR_rt_sigqueueinfo, getpid(), SIGWINCH, si);
> > return 0;
> > }
> > $ gcc -g sigqueue_test.c && ./a.out && ~/valgrind/trunk/inst/bin/valgrind
> > ./a.out
> > sizeof(*si) = 128
> > 0 4 8 16
> > ==13294== Memcheck, a memory error detector
> > ==13294== Copyright (C) 2002-2009, and GNU GPL'd, by Julian Seward et al.
> > ==13294== Using Valgrind-3.6.0.SVN and LibVEX; rerun with -h for
> copyright
> > info
> > ==13294== Command: ./a.out
> > ==13294==
> > sizeof(*si) = 128
> > 0 4 8 16
> > ==13294== Syscall param rt_sigqueueinfo(uinfo) points to unaddressable
> > byte(s)
> > ==13294== at 0x4EE82E9: syscall (in /usr/grte/v1/lib64/libc-2.3.6.so)
> > ==13294== by 0x400750: main (sigqueue_test.c:24)
> > ==13294== Address 0x516a0c0 is 0 bytes after a block of size 128 alloc'd
> > ==13294== at 0x4C1BE27: malloc (vg_replace_malloc.c:195)
> > ==13294== by 0x4006DB: main (sigqueue_test.c:18)
> > ==13294==
> > ==13294==
> > ==13294== HEAP SUMMARY:
> > ==13294== in use at exit: 128 bytes in 1 blocks
> > ==13294== total heap usage: 1 allocs, 0 frees, 128 bytes allocated
> > ==13294==
> > ==13294== LEAK SUMMARY:
> > ==13294== definitely lost: 128 bytes in 1 blocks
> > ==13294== indirectly lost: 0 bytes in 0 blocks
> > ==13294== possibly lost: 0 bytes in 0 blocks
> > ==13294== still reachable: 0 bytes in 0 blocks
> > ==13294== suppressed: 0 bytes in 0 blocks
> > ==13294== Rerun with --leak-check=full to see details of leaked memory
> > ==13294==
> > ==13294== For counts of detected and suppressed errors, rerun with: -v
> > ==13294== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 5 from 5)
> >
> >
> > Fix (something like):
> > Index: include/vki/vki-linux.h
> > ===================================================================
> > --- include/vki/vki-linux.h (revision 10904)
> > +++ include/vki/vki-linux.h (working copy)
> > @@ -340,16 +340,17 @@
> > void __user *sival_ptr;
> > } vki_sigval_t;
> > -#ifndef __VKI_ARCH_SI_PREAMBLE_SIZE
> > -#define __VKI_ARCH_SI_PREAMBLE_SIZE (3 * sizeof(int))
> > -#endif
> > -
> > #define VKI_SI_MAX_SIZE 128
> > #ifndef VKI_SI_PAD_SIZE
> > -#define VKI_SI_PAD_SIZE ((VKI_SI_MAX_SIZE -
> > __VKI_ARCH_SI_PREAMBLE_SIZE) / sizeof(int))
> > +# if defined (VGA_amd64) // or whatever is right for 64-bit arch.
> > +# define VKI_SI_PAD_SIZE ((VKI_SI_MAX_SIZE / sizeof (int)) - 4)
> > +# else
> > +# define VKI_SI_PAD_SIZE ((VKI_SI_MAX_SIZE / sizeof (int)) - 3)
> > +# endif
> > #endif
> > +
> > #ifndef __VKI_ARCH_SI_UID_T
> > #define __VKI_ARCH_SI_UID_T vki_uid_t
> > #endif
>
> I have added a modified version of your test program to the repository
> and have filed this issue in bugzilla
> (https://bugs.kde.org/show_bug.cgi?id=212064). But I'm not sure that
> the proposed fix makes sense. The proposed modifications do not match
> the definitions in the kernel header files (<asm-generic/siginfo.h>).
>
I took the code from /usr/include/bits/siginfo.h
>
> Bart.
>
|
|
From: Tom H. <to...@co...> - 2009-10-27 15:47:46
|
On 27/10/09 15:22, Konstantin Serebryany wrote: > I have added a modified version of your test program to the repository > and have filed this issue in bugzilla > (https://bugs.kde.org/show_bug.cgi?id=212064). But I'm not sure that > the proposed fix makes sense. The proposed modifications do not match > the definitions in the kernel header files (<asm-generic/siginfo.h>). > > > I took the code from /usr/include/bits/siginfo.h Which is a userland definition - valgrind deals with the system call interface so need to work with kernel definitions. Basically somebody will need to work out what magic is going on between glibc and the kernel that allows the two to have different expectations of the size. Tom -- Tom Hughes (to...@co...) http://www.compton.nu/ |
|
From: Bart V. A. <bar...@gm...> - 2009-10-27 16:18:26
|
On Tue, Oct 27, 2009 at 4:47 PM, Tom Hughes <to...@co...> wrote: > On 27/10/09 15:22, Konstantin Serebryany wrote: > >> I have added a modified version of your test program to the repository >> and have filed this issue in bugzilla >> (https://bugs.kde.org/show_bug.cgi?id=212064). But I'm not sure that >> the proposed fix makes sense. The proposed modifications do not match >> the definitions in the kernel header files (<asm-generic/siginfo.h>). >> >> >> I took the code from /usr/include/bits/siginfo.h > > Which is a userland definition - valgrind deals with the system call > interface so need to work with kernel definitions. > > Basically somebody will need to work out what magic is going on between > glibc and the kernel that allows the two to have different expectations of > the size. The rt_sigqueueinfo wrapper in coregrind/m_syswrap/syswrap-linux.c contains a.o. the following statement: PRE_MEM_READ( "rt_sigqueueinfo(uinfo)", ARG3, sizeof(vki_siginfo_t) ); Apparently the size of the vki_siginfo_t structure defined in Valgrind's header files is larger than VKI_SI_MAX_SIZE. This doesn't make sense. I found the following comment in include/vki/vki-linux.h, of which I'm not sure that it is correct: ... // [[Nb: this type changed between 2.4 and 2.6, but not in a way that // affects Valgrind.]] typedef struct vki_siginfo { ... Bart. |