|
From: Bobby K. <rd...@kr...> - 2008-02-27 16:12:11
|
Hi!
I recently tried to debug my multi-threaded app using Valgrind 3.2.3
on 32-bit linux/x86 and am baffled by the problems it finds in my
code. I've poured over the examples and tutorials but cant seem to
make the connection to how my code is causing these problems.
Essentially, myy code reads data from a network, xml decodes it,
processes, and sends a reply back.
Here is one error message.
==25662== Use of uninitialised value of size 4
==25662== at 0x5FFDD7: ____strtol_l_internal (in /lib/libc-2.7.so)
==25662== by 0x5FFB2F: strtol (in /lib/libc-2.7.so)
==25662== by 0x5FCE20: atoi (in /lib/libc-2.7.so)
==25662== by 0x805C265: RecvMessage (session.c:579)
==25662== by 0x804E991: InSessionHandler (d.c:730)
==25662== by 0x76450A: start_thread (in /lib/libpthread-2.7.so)
==25662== by 0x6A5B2D: clone (in /lib/libc-2.7.so)
/* figure out length of message from wireheader */
msgLenStr[0] = 0;
strncpy(msgLenStr,wh.msgLenStr,10);
msgLenStr[10] = 0;
msgLen = atoi(msgLenStr);
I'm not sure I understand why the code above is tripping valgrind.
The structure wh is read from SSL_read which in turn reads it from the
socket. I've tried bzero'ing it prior to the socket read too.
Here is another baffling message about the same snippet of code.
==25662== Conditional jump or move depends on uninitialised value(s)
==25662== at 0x5FFDFA: ____strtol_l_internal (in /lib/libc-2.7.so)
==25662== by 0x5FFB2F: strtol (in /lib/libc-2.7.so)
==25662== by 0x5FCE20: atoi (in /lib/libc-2.7.so)
==25662== by 0x805C265: RecvMessage (session.c:579)
==25662== by 0x804E991: InSessionHandler (d.c:730)
==25662== by 0x76450A: start_thread (in /lib/libpthread-2.7.so)
==25662== by 0x6A5B2D: clone (in /lib/libc-2.7.so)
Same code. My code is littered with these error messages.
Any pointers or suggestions on how to figure out what I'm doing wrong
to trip this error message?
Thanks,
Bobby
|
|
From: Christoph B. <bar...@or...> - 2008-02-27 16:18:38
|
Am Mittwoch, 27. Februar 2008 schrieb Bobby Krupczak:
> ==25662== Use of uninitialised value of size 4
> ==25662== at 0x5FFDD7: ____strtol_l_internal (in /lib/libc-2.7.so)
> ==25662== by 0x5FFB2F: strtol (in /lib/libc-2.7.so)
> ==25662== by 0x5FCE20: atoi (in /lib/libc-2.7.so)
> ==25662== by 0x805C265: RecvMessage (session.c:579)
> ==25662== by 0x804E991: InSessionHandler (d.c:730)
> ==25662== by 0x76450A: start_thread (in /lib/libpthread-2.7.so)
> ==25662== by 0x6A5B2D: clone (in /lib/libc-2.7.so)
>
> /* figure out length of message from wireheader */
> msgLenStr[0] = 0;
> strncpy(msgLenStr,wh.msgLenStr,10);
> msgLenStr[10] = 0;
> msgLen = atoi(msgLenStr);
>
> I'm not sure I understand why the code above is tripping valgrind.
> The structure wh is read from SSL_read which in turn reads it from the
> socket. I've tried bzero'ing it prior to the socket read too.
>
> Here is another baffling message about the same snippet of code.
>
How do you fill your wh.msgLenStr?
Try the following in front of the strncpy:
for (int i = 0; i < 10; ++i) printf("%d ", (int) wh.msgLenStr[i]);
|
|
From: Bobby K. <rd...@kr...> - 2008-02-27 16:23:32
|
Hi!
> Am Mittwoch, 27. Februar 2008 schrieb Bobby Krupczak:
>
> > ==25662== Use of uninitialised value of size 4
> > ==25662== at 0x5FFDD7: ____strtol_l_internal (in /lib/libc-2.7.so)
> > ==25662== by 0x5FFB2F: strtol (in /lib/libc-2.7.so)
> > ==25662== by 0x5FCE20: atoi (in /lib/libc-2.7.so)
> > ==25662== by 0x805C265: RecvMessage (session.c:579)
> > ==25662== by 0x804E991: InSessionHandler (d.c:730)
> > ==25662== by 0x76450A: start_thread (in /lib/libpthread-2.7.so)
> > ==25662== by 0x6A5B2D: clone (in /lib/libc-2.7.so)
> >
> > /* figure out length of message from wireheader */
> > msgLenStr[0] = 0;
> > strncpy(msgLenStr,wh.msgLenStr,10);
> > msgLenStr[10] = 0;
> > msgLen = atoi(msgLenStr);
> >
> > I'm not sure I understand why the code above is tripping valgrind.
> > The structure wh is read from SSL_read which in turn reads it from the
> > socket. I've tried bzero'ing it prior to the socket read too.
> >
> > Here is another baffling message about the same snippet of code.
> >
>
> How do you fill your wh.msgLenStr?
>
> Try the following in front of the strncpy:
>
> for (int i = 0; i < 10; ++i) printf("%d ", (int) wh.msgLenStr[i]);
I bzero'd it and then SSL_read reads into the structure as well.
Thats why I'm so confused about why I'm receiving this error message.
I'm receiving 20-30 similar warnings for essentially the same type of
thing -- SSL reading from socket and then parsing the data is tripping
valgrind.
I'll give the above a whirl.
Thanks,
Bobby
|
|
From: Bobby K. <rd...@kr...> - 2008-02-27 19:03:09
|
Hi!
> > /* figure out length of message from wireheader */
> > msgLenStr[0] = 0;
> > strncpy(msgLenStr,wh.msgLenStr,10);
> > msgLenStr[10] = 0;
> > msgLen = atoi(msgLenStr);
> >
> > I'm not sure I understand why the code above is tripping valgrind.
> > The structure wh is read from SSL_read which in turn reads it from the
> > socket. I've tried bzero'ing it prior to the socket read too.
> >
> > Here is another baffling message about the same snippet of code.
> >
>
> How do you fill your wh.msgLenStr?
>
> Try the following in front of the strncpy:
>
> for (int i = 0; i < 10; ++i) printf("%d ", (int) wh.msgLenStr[i]);
So, I put the code in that you suggested and it too trips valgrind to
say that I'm using uninitialized data.
==26326== Conditional jump or move depends on uninitialised value(s)
==26326== at 0x60F59B: vfprintf (in /lib/libc-2.7.so)
==26326== by 0x6179B2: printf (in /lib/libc-2.7.so)
==26326== by 0x805C266: xmpRecvMessage (xmp_session.c:578)
==26326== by 0x804E991: xmpdInSessionHandler (xmpd.c:730)
==26326== by 0x76450A: start_thread (in /lib/libpthread-2.7.so)
==26326== by 0x6A5B2D: clone (in /lib/libc-2.7.so)
Here are the values:
52 54 54 0 0 0 0 0 0 0
The values are essentially saying that I have 466 bytes of data
following my wireheader.
I'm perplexed as to why these warnings are being triggered.
I'm using TCP over SSL and transmitting xml-encoded packets. I'm
using libxml to encode/decode. Could this be related?
Thanks,
Bobby
|
|
From: Christoph B. <bar...@or...> - 2008-02-27 21:56:01
|
Am Mittwoch, 27. Februar 2008 schrieb Bobby Krupczak: > So, I put the code in that you suggested and it too trips valgrind to > say that I'm using uninitialized data. > > ==26326== Conditional jump or move depends on uninitialised value(s) > ==26326== at 0x60F59B: vfprintf (in /lib/libc-2.7.so) > ==26326== by 0x6179B2: printf (in /lib/libc-2.7.so) > ==26326== by 0x805C266: xmpRecvMessage (xmp_session.c:578) > ==26326== by 0x804E991: xmpdInSessionHandler (xmpd.c:730) > ==26326== by 0x76450A: start_thread (in /lib/libpthread-2.7.so) > ==26326== by 0x6A5B2D: clone (in /lib/libc-2.7.so) > > Here are the values: > > 52 54 54 0 0 0 0 0 0 0 > > The values are essentially saying that I have 466 bytes of data > following my wireheader. > > I'm perplexed as to why these warnings are being triggered. > > I'm using TCP over SSL and transmitting xml-encoded packets. I'm > using libxml to encode/decode. Could this be related? > The question is now: How do you read the values? And which byte is undefined? Show the code that fills the array. |
|
From: Bobby K. <rd...@kr...> - 2008-02-27 22:26:58
|
Hi!
> > ==26326== Conditional jump or move depends on uninitialised value(s)
> > ==26326== at 0x60F59B: vfprintf (in /lib/libc-2.7.so)
> > ==26326== by 0x6179B2: printf (in /lib/libc-2.7.so)
> > ==26326== by 0x805C266: RecvMessage (session.c:578)
> > ==26326== by 0x804E991: InSessionHandler (d.c:730)
> > ==26326== by 0x76450A: start_thread (in /lib/libpthread-2.7.so)
> > ==26326== by 0x6A5B2D: clone (in /lib/libc-2.7.so)
> >
> > Here are the values:
> >
> > 52 54 54 0 0 0 0 0 0 0
> >
> > The values are essentially saying that I have 466 bytes of data
> > following my wireheader.
> >
> > I'm perplexed as to why these warnings are being triggered.
> >
> > I'm using TCP over SSL and transmitting xml-encoded packets. I'm
> > using libxml to encode/decode. Could this be related?
> >
>
> The question is now: How do you read the values? And which byte is
> > undefined?
> Show the code that fills the array.
The code to read data into the buffer is essentially an SSL_read. In
all the cases of the errors I'm getting, data is being placed into the
buffer by calls to network I/O.
typedef struct xmpWireHdr {
/* ascii length of string no more than 10
chars; may or may not be zero terminated
and strings must be in decimal */
char msgLenStr[10];
/* user, authen-info here, parameters, etc. XXX */
} xmpWireHdr_t;
/* read in the wireheader waiting until we've read the entire structure */
bzero((void *)&wh,sizeof(wh)); /* make valgrind happy? */
ret = 0; nread = 0;
bufptr = (u_char *)&wh;
for (nread = 0; nread < sizeof(wh); nread += ret) {
ret = SSL_read(ssl,bufptr+nread,sizeof(wh)-nread);
if (ret <= 0)
break;
}
if (ret == 0) {
if (session_verbose)
fprintf(stderr,"RecvMessage: SSL shutdown during wirehdr read\n");
Stats.inErrors++;
return -1;
}
if (ret < 0) {
if (session_verbose)
fprintf(stderr,"RecvMessage: SSL read error, %d\n",
SSL_get_error(ssl,ret));
Stats.inErrors++;
return -1;
}
/* figure out length of message from wireheader */
msgLenStr[0] = 0; /* valgrind happy? */
strncpy(msgLenStr,wh.msgLenStr,10);
msgLenStr[10] = 0;
msgLen = atoi(msgLenStr);
[I've been digging on the net and searching the mailing list archives
and reading about buffers and initializing portions of the buffer but
accessing past that and fooling valgrind.]
How can I figure out which byte that memcheck is complaining about?
Also, wouldnt a bzero of the structure, prior to the call to SSL_read,
essentially initialize the buffer and make valgrind happy?
Thanks,
Bobby
|
|
From: Christoph B. <bar...@or...> - 2008-02-27 22:39:09
|
Am Mittwoch, 27. Februar 2008 schrieb Bobby Krupczak:
> Hi!
>
> > > ==26326== Conditional jump or move depends on uninitialised value(s)
> > > ==26326== at 0x60F59B: vfprintf (in /lib/libc-2.7.so)
> > > ==26326== by 0x6179B2: printf (in /lib/libc-2.7.so)
> > > ==26326== by 0x805C266: RecvMessage (session.c:578)
> > > ==26326== by 0x804E991: InSessionHandler (d.c:730)
> > > ==26326== by 0x76450A: start_thread (in /lib/libpthread-2.7.so)
> > > ==26326== by 0x6A5B2D: clone (in /lib/libc-2.7.so)
> > >
> > > Here are the values:
> > >
> > > 52 54 54 0 0 0 0 0 0 0
> > >
> > > The values are essentially saying that I have 466 bytes of data
> > > following my wireheader.
> > >
> > > I'm perplexed as to why these warnings are being triggered.
> > >
> > > I'm using TCP over SSL and transmitting xml-encoded packets. I'm
> > > using libxml to encode/decode. Could this be related?
> >
> > The question is now: How do you read the values? And which byte is
> >
> > > undefined?
> >
> > Show the code that fills the array.
>
> The code to read data into the buffer is essentially an SSL_read. In
> all the cases of the errors I'm getting, data is being placed into the
> buffer by calls to network I/O.
>
> typedef struct xmpWireHdr {
>
> /* ascii length of string no more than 10
> chars; may or may not be zero terminated
> and strings must be in decimal */
>
> char msgLenStr[10];
>
> /* user, authen-info here, parameters, etc. XXX */
>
> } xmpWireHdr_t;
>
>
> /* read in the wireheader waiting until we've read the entire
> structure */ bzero((void *)&wh,sizeof(wh)); /* make valgrind happy? */
> ret = 0; nread = 0;
> bufptr = (u_char *)&wh;
> for (nread = 0; nread < sizeof(wh); nread += ret) {
> ret = SSL_read(ssl,bufptr+nread,sizeof(wh)-nread);
> if (ret <= 0)
> break;
> }
> if (ret == 0) {
> if (session_verbose)
> fprintf(stderr,"RecvMessage: SSL shutdown during wirehdr read\n");
> Stats.inErrors++;
> return -1;
> }
> if (ret < 0) {
> if (session_verbose)
> fprintf(stderr,"RecvMessage: SSL read error, %d\n",
> SSL_get_error(ssl,ret));
> Stats.inErrors++;
> return -1;
> }
>
> /* figure out length of message from wireheader */
> msgLenStr[0] = 0; /* valgrind happy? */
>
> strncpy(msgLenStr,wh.msgLenStr,10);
> msgLenStr[10] = 0;
> msgLen = atoi(msgLenStr);
>
> How can I figure out which byte that memcheck is complaining about?
1. The byte that is written after the first complaint is the bad byte.
2. Is your msgLenStr really 11 bytes long?
3. You use a fixed length ascii string for the length of the message?
4. I guess wh is of type xmpWireHdr_t. Then I would say that the way you read
the data in is evil :).
5. Try to validate the data right after the SSL_read. You could use the
following loop:
for (int i = 0; i < ret; ++i) {
if (VALGRIND_CHECK_VALUE_IS_DEFINED(bufptr[nread + i]) {
printf("Byte %d is not defined\n", nread + i);
}
}
Christoph
|
|
From: Bobby K. <rd...@kr...> - 2008-02-28 00:05:43
|
Hi!
> > char msgLenStr[10];
> > }
> >
> > /* figure out length of message from wireheader */
> > msgLenStr[0] = 0; /* valgrind happy? */
> >
> > strncpy(msgLenStr,wh.msgLenStr,10);
> > msgLenStr[10] = 0;
> > msgLen = atoi(msgLenStr);
Setting byte at index 10 is bad since C arrays start at index 0 :)
I'm glad you saw this. I cant tell you how many times I've looked
over this code . . . Sigh.
However, when I added the for loop that you suggested, I added it
prior to writing at index [10] and valgrind still complained.
> 2. Is your msgLenStr really 11 bytes long?
Nope. Thanks for catching this.
> 3. You use a fixed length ascii string for the length of the
> message?
For a variety of complex reasons due mostly to portability across
architectures and platforms, I send a wire header first before the
actual xml-message. It makes parsing xml a lot easier. Also, the
wire header will eventually be expanded to include
application/authentication information above and beyond what ssl
provides me.
> 4. I guess wh is of type xmpWireHdr_t. Then I would say that the
> way you read
Yes, sorry.
> the data in is evil :).
I'm used to it. The approach I'm using is not great but it does work.
Its not unlike many Internet protocols that are in ascii (e.g. http).
> for (int i = 0; i < ret; ++i) {
> if (VALGRIND_CHECK_VALUE_IS_DEFINED(bufptr[nread + i]) {
> printf("Byte %d is not defined\n", nread + i);
> }
> }
I'll give this a whirl and see what it says. I'll also fix my bug
. .
Thanks!
Bobby
|
|
From: Christoph B. <bar...@or...> - 2008-02-28 00:47:57
|
Am Mittwoch, 27. Februar 2008 schrieben Sie:
>
> typedef struct xmpWireHdr {
>
> /* ascii length of string no more than 10
> chars; may or may not be zero terminated
> and strings must be in decimal */
>
> char msgLenStr[10];
>
> /* user, authen-info here, parameters, etc. XXX */
>
> } xmpWireHdr_t;
>
How long is the message length string as it is defined in the protocol? What
is the specified format?
|
|
From: Bobby K. <rd...@kr...> - 2008-02-28 01:13:23
|
Hi!
> Am Mittwoch, 27. Februar 2008 schrieben Sie:
> >
> > typedef struct xmpWireHdr {
> >
> > /* ascii length of string no more than 10
> > chars; may or may not be zero terminated
> > and strings must be in decimal */
> >
> > char msgLenStr[10];
> >
> > /* user, authen-info here, parameters, etc. XXX */
> >
> > } xmpWireHdr_t;
> >
>
> How long is the message length string as it is defined in the protocol? What
> is the specified format?
Turns out its not a bug on my part. Rats. I was hoping that would
help solve my puzzle.
The wireheader has a field of 10-characters. The protocol says to put
the length of the PDU, in ascii chars, in this field. When the
protocol entity writes it, it writes the whole ten bytes so the
receiving entity should be reading 10 bytes as well.
The msgLengStr[] string I copy it into locally was declared of length
11 so using index 10 was OK and thus my false alarm.
I am trying to add the macro you suggested now to check the data right
after it comes out of ssl-read.
I also get a ton of similar valgrind errors when I read the actual pdu
out of the socket via SSL_read and then use libxml to parse the pdu.
I get all kinds of warnings. My suspicion is if I solve this smaller
problem, I'll figure out the bigger problem.
Is there something with SSL though that triggers this? I wish I had
tested with valgrind *before* I added SSL to my code. Right now it
would be too ugly to break it back apart.
Thanks,
Bobby
|