struct tcphdr defines the tcp/ip standard header. Following that is up to 40 bytes of 'options'. Since the def of tp is 'struct tcphdr tp;', any reference using it into the opts will be seen as an uninitialized reference.
I've been routinely applying Chris' patch for the un-aligned moves/sparc problem, and that may be what Valgrind is catching. He changes the definitions to: - u_char *opt_ptr = tcp_opt; + u_char *opt_ptr = (u_char *)((struct tcphdr*)&tp + 1); + u_char *tcp_data = + (u_char *)((struct tcphdr*)&tp + tp.th_off * 4); I just don't know - it's not clear how deep valgrind goes with it's analysis - it could be that it sees the 24 byte header being overflowed or it could be picking up something else about the code. Since you're going to be in there anyway, looking at Chris' patch, I wanted you to look at it and make sure the code is ok. Once you're sure that the code is right, I'll try and code a suppress for Valgrind for this. -----Burton -----Original Message----- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] Behalf Of Luca Deri Sent: Thursday, September 11, 2003 3:53 AM To: [EMAIL PROTECTED] Subject: Re: [Ntop-dev] VALGRIND bug report Burton, I can't follow you here. opt_ptr is the problem right? If so: struct tcphdr *tcp = (struct tcphdr*)(bp+hlen); u_char *tcp_opt = (u_char *)(tcp + 1); u_char *opt_ptr = tcp_opt; In the above statements I can't see tp being used at all, so I don't understand the memcpy problem. Please explain. Rgds, Luca Burton M. Strauss III wrote: >==922== Thread 2: >==922== Conditional jump or move depends on uninitialised value(s) >==922== at 0x402AA52A: processIpPkt (pbuf.c:975) >==922== by 0x402AD189: processPacket (pbuf.c:2513) >==922== by 0x402AB37C: dequeuePacket (pbuf.c:1735) >==922== by 0x4045B5C2: thread_wrapper (vg_libpthread.c:667) >==922== >==922== Thread 2: >==922== Use of uninitialised value of size 4 >==922== at 0x402AA536: processIpPkt (pbuf.c:975) >==922== by 0x402AD189: processPacket (pbuf.c:2513) >==922== by 0x402AB37C: dequeuePacket (pbuf.c:1735) >==922== by 0x4045B5C2: thread_wrapper (vg_libpthread.c:667) > > > if(srcHost->fingerprint == NULL) { > char fingerprint[64]; > int WIN=0, MSS=-1, WS=-1, S=0, N=0, D=0, T=0; > int ttl; > char WSS[3], _MSS[5]; > > if (tp.th_flags & TH_SYN) /* only SYN or SYN-2ACK packets */ > { >... > while(opt_ptr < tcp_data) > { > > >>>> switch(*opt_ptr) >>>> >>>> > { >... > } > } > } >... > } > > >Luca, I think valgrind is right, because the set of tp is this: > > memcpy(&tp, bp+hlen, sizeof(struct tcphdr)); > >(pbuf.c @ 921), which doesn't copy any of the option fields. We should >probably add 40 bytes (that's the maximum length of the options field) to >that memcpy... > >-----Burton > >_______________________________________________ >Ntop-dev mailing list >[EMAIL PROTECTED] >http://listgateway.unipi.it/mailman/listinfo/ntop-dev > > -- Luca Deri <[EMAIL PROTECTED]> http://luca.ntop.org/ Hacker: someone who loves to program and enjoys being clever about it - Richard Stallman _______________________________________________ Ntop-dev mailing list [EMAIL PROTECTED] http://listgateway.unipi.it/mailman/listinfo/ntop-dev _______________________________________________ Ntop-dev mailing list [EMAIL PROTECTED] http://listgateway.unipi.it/mailman/listinfo/ntop-dev
