[OpenVPN home] [Date Prev] [Date Index] [Date Next]
[OpenVPN mailing lists] [Thread Prev] [Thread Index] [Thread Next]
Google
 
Web openvpn.net

Re: [Openvpn-users] v2.0 hangs


  • Subject: Re: [Openvpn-users] v2.0 hangs
  • From: "James Yonan" <jim@xxxxxxxxx>
  • Date: Thu, 6 May 2004 16:04:56 -0000

Timo Sirainen <tss@xxxxxx> said:

> On Thu, 2004-05-06 at 07:54, James Yonan wrote:
> > > ==3026== Conditional jump or move depends on uninitialised value(s)
> > > ==3026==    at 0x4002496B: strlen (mac_replace_strmem.c:162)
> > > ==3026==    by 0x804BA02: string_alloc (buffer.c:341)
> > > ==3026==    by 0x806E279: verify_callback (ssl.c:510)
> > > ==3026==    by 0x40322BE4: (within 
> > > /usr/lib/i686/cmov/libcrypto.so.0.9.7)
> 
> Probably fixed with:
> 
> --- ssl.c.old	2004-05-06 17:52:46.000000000 +0300
> +++ ssl.c	2004-05-06 17:53:00.000000000 +0300
> @@ -505,6 +505,8 @@
>      char common_name[TLS_CN_LEN];
>      if (ctx->error_depth == 0)
>        extract_common_name (common_name, TLS_CN_LEN, subject);
> +    else
> +      common_name[0] = '\0';
>      if (session->common_name)
>        free (session->common_name);
>      session->common_name = string_alloc (common_name, NULL);

You are right, common_name could potentially be uninitialized, though what we
really want is:

  /* save common name in session object */
  if (ctx->error_depth == 0)
    {
      char common_name[TLS_CN_LEN];
      extract_common_name (common_name, TLS_CN_LEN, subject);
      if (session->common_name)
	free (session->common_name);
      session->common_name = string_alloc (common_name, NULL);
    }

because we are mostly interested in the common name when error_depth == 0.  We
also want session->common_name to remain NULL unless it points to a real
common name (and not just an empty string).  Will be fixed in test27.

> > > ==3026== Syscall param socketcall.sendto(msg) contains uninitialised or 
> > > unaddressable byte(s)
> > > ==3026==    at 0x40466456: __libc_sendto (in /lib/libc-2.3.2.so)
> > > ==3026==    by 0x805D3A1: tunnel_point_to_point (openvpn.c:81)
> > > ==3026==    by 0x805D5B7: main (openvpn.c:163)
> > > ==3026==  Address 0x41B417F6 is 98 bytes inside a block of size 1579 
> ..
> 
> I'm not sure about this as it's skipping some of the functions before
> sendto()..

Basically you have a call graph like tunnel_point_to_point -> process_io ->
process_outgoing_link -> link_socket_write -> link_socket_write_udp ->
link_socket_write_udp_posix -> sendto

I don't see any uninitialized data on first glance.  There are some ASSERT
statements though.  Does valgrind know that certain functions like ASSERT or
msg (M_FATAL, ...) might never return?

> > > And lots of problems inside libcrypto. I'm not sure if they're real or 
> > > not.
> > 
> > These tend to look like false alarms.
> 
> Valgrind usually shows pretty correctly if there's problems. Sometimes
> it just reports the errors in a bit wrong place or without enough
> information.

I haven't looked in detail at the libcrypto code (essentially the non-SSL part
of OpenSSL), it's certainly worth looking at if you think the problems are real.

James