Bug 19416 - resource leak in function _dbus_connect_tcp_socket
Summary: resource leak in function _dbus_connect_tcp_socket
Status: RESOLVED FIXED
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: 1.2.x
Hardware: x86 (IA32) Windows (All)
: medium normal
Assignee: Havoc Pennington
QA Contact: John (J5) Palmieri
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-01-05 23:29 UTC by zhangshichang
Modified: 2010-08-08 17:58 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
related patch by Romain Pokrzywka (1.00 KB, patch)
2010-08-08 17:57 UTC, Ralf Habacker
Details | Splinter Review

Description zhangshichang 2009-01-05 23:29:14 UTC
in the follow function,the fd from socket function is not be close.
i think that the closesocket must be called.otherwise, resource leak

int
_dbus_connect_tcp_socket (const char     *host,
                          const char     *port,
                          const char     *family,
                          DBusError      *error)
{
  int fd = -1, res;
  struct addrinfo hints;
  struct addrinfo *ai, *tmp;

  _DBUS_ASSERT_ERROR_IS_CLEAR (error);

  _dbus_win_startup_winsock ();

  fd = socket (AF_INET, SOCK_STREAM, 0);

  if (DBUS_SOCKET_IS_INVALID (fd))
    {
      DBUS_SOCKET_SET_ERRNO ();
      dbus_set_error (error,
                      _dbus_error_from_errno (errno),
                      "Failed to create socket: %s",
                      _dbus_strerror (errno));

      return -1;
    }

  _DBUS_ASSERT_ERROR_IS_CLEAR(error);

  _DBUS_ZERO (hints);

  if (!family)
    hints.ai_family = AF_UNSPEC;
  else if (!strcmp(family, "ipv4"))
    hints.ai_family = AF_INET;
  else if (!strcmp(family, "ipv6"))
    hints.ai_family = AF_INET6;
  else
    {
      dbus_set_error (error,
                      _dbus_error_from_errno (errno),
                      "Unknown address family %s", family);
      return -1;
    }
  fprintf(stderr, "Family %s\n", family ? family : "none");
  hints.ai_protocol = IPPROTO_TCP;
  hints.ai_socktype = SOCK_STREAM;
#ifdef AI_ADDRCONFIG
  hints.ai_flags = AI_ADDRCONFIG;
#else
  hints.ai_flags = 0;
#endif

  if ((res = getaddrinfo(host, port, &hints, &ai)) != 0)
    {
      dbus_set_error (error,
                      _dbus_error_from_errno (errno),
                      "Failed to lookup host/port: \"%s:%s\": %s (%d)",
                      host, port, gai_strerror(res), res);
      closesocket (fd);
      return -1;
    }

  tmp = ai;
  while (tmp)
    {
      if ((fd = socket (tmp->ai_family, SOCK_STREAM, 0)) < 0)
        {
          freeaddrinfo(ai);
          dbus_set_error(error,
                         _dbus_error_from_errno (errno),
                         "Failed to open socket: %s",
                         _dbus_strerror (errno));
          return -1;
        }
      _DBUS_ASSERT_ERROR_IS_CLEAR(error);

      if (connect (fd, (struct sockaddr*) tmp->ai_addr, tmp->ai_addrlen) < 0)
        {
          closesocket(fd);
          fd = -1;
          tmp = tmp->ai_next;
          continue;
        }

      break;
    }
  freeaddrinfo(ai);

  if (fd == -1)
    {
      dbus_set_error (error,
                      _dbus_error_from_errno (errno),
                      "Failed to connect to socket \"%s:%s\" %s",
                      host, port, _dbus_strerror(errno));
      return -1;
    }


  if (!_dbus_set_fd_nonblocking (fd, error))
    {
      closesocket (fd);
      fd = -1;

      return -1;
    }

  return fd;
}
Comment 1 John (J5) Palmieri 2009-01-06 09:04:09 UTC
I'm not a windows developer but I do see some potential problems that may be broken or may just be how windows work.

Havoc do you know who we can assign this to?  I've inlined some comments bellow prepended by J5: for easy searching.

(In reply to comment #0)
> in the follow function,the fd from socket function is not be close.
> i think that the closesocket must be called.otherwise, resource leak
> 
> int
> _dbus_connect_tcp_socket (const char     *host,
>                           const char     *port,
>                           const char     *family,
>                           DBusError      *error)
> {
>   int fd = -1, res;
>   struct addrinfo hints;
>   struct addrinfo *ai, *tmp;
> 
>   _DBUS_ASSERT_ERROR_IS_CLEAR (error);
> 
>   _dbus_win_startup_winsock ();
> 
>   fd = socket (AF_INET, SOCK_STREAM, 0);
> 
>   if (DBUS_SOCKET_IS_INVALID (fd))
>     {
>       DBUS_SOCKET_SET_ERRNO ();
>       dbus_set_error (error,
>                       _dbus_error_from_errno (errno),
>                       "Failed to create socket: %s",
>                       _dbus_strerror (errno));
> 
>       return -1;
>     }
> 
>   _DBUS_ASSERT_ERROR_IS_CLEAR(error);
> 
>   _DBUS_ZERO (hints);
> 
>   if (!family)
>     hints.ai_family = AF_UNSPEC;
>   else if (!strcmp(family, "ipv4"))
>     hints.ai_family = AF_INET;
>   else if (!strcmp(family, "ipv6"))
>     hints.ai_family = AF_INET6;
>   else
>     {
>       dbus_set_error (error,
>                       _dbus_error_from_errno (errno),
>                       "Unknown address family %s", family);
        /* J5: should close the socket here */ 
>       return -1;
>     }
>   fprintf(stderr, "Family %s\n", family ? family : "none");
>   hints.ai_protocol = IPPROTO_TCP;
>   hints.ai_socktype = SOCK_STREAM;
> #ifdef AI_ADDRCONFIG
>   hints.ai_flags = AI_ADDRCONFIG;
> #else
>   hints.ai_flags = 0;
> #endif
> 
>   if ((res = getaddrinfo(host, port, &hints, &ai)) != 0)
>     {
>       dbus_set_error (error,
>                       _dbus_error_from_errno (errno),
>                       "Failed to lookup host/port: \"%s:%s\": %s (%d)",
>                       host, port, gai_strerror(res), res);
>       closesocket (fd);
>       return -1;
>     }
> 
>   tmp = ai;

   /* J5: it seems that fd is being dangled here and the socket 
          should be closed before going into the while loop
   */
>   while (tmp)
>     {
>       if ((fd = socket (tmp->ai_family, SOCK_STREAM, 0)) < 0)
>         {
>           freeaddrinfo(ai);
>           dbus_set_error(error,
>                          _dbus_error_from_errno (errno),
>                          "Failed to open socket: %s",
>                          _dbus_strerror (errno));
>           return -1;
>         }
>       _DBUS_ASSERT_ERROR_IS_CLEAR(error);
> 
>       if (connect (fd, (struct sockaddr*) tmp->ai_addr, tmp->ai_addrlen) < 0)
>         {
>           closesocket(fd);
>           fd = -1;
>           tmp = tmp->ai_next;
>           continue;
>         }
> 
>       break;
>     }
>   freeaddrinfo(ai);
> 
>   if (fd == -1)
>     {
>       dbus_set_error (error,
>                       _dbus_error_from_errno (errno),
>                       "Failed to connect to socket \"%s:%s\" %s",
>                       host, port, _dbus_strerror(errno));
>       return -1;
>     }
> 
> 
>   if (!_dbus_set_fd_nonblocking (fd, error))
>     {
>       closesocket (fd);
>       fd = -1;
> 
>       return -1;
>     }
> 
>   return fd;
> }
> 

Comment 2 Havoc Pennington 2009-01-07 06:55:23 UTC
I don't know of anyone to assign it to.
Comment 3 Ralf Habacker 2010-08-08 17:57:47 UTC
Created attachment 37710 [details] [review]
related patch by Romain Pokrzywka
Comment 4 Ralf Habacker 2010-08-08 17:58:15 UTC
patch applied to dbus master


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.