[Nagiosplug-devel] [ nagiosplug-Patches-1075725 ] PATCH for netutils.c (HEAD): fix SEGV when connect fails.

[SourceForge.net]

Patches item #1075725, was opened at 2004-11-30 02:34
Message generated for change (Comment added) made by stanleyhopcroft
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=397599&aid=1075725&group_id=29880

Category: Bugfix
Group: None
Status: Open
Resolution: None
Priority: 5
Submitted By: Stanley Hopcroft (stanleyhopcroft)
Assigned to: Benoit Mortier (opensides)
Summary: PATCH for netutils.c (HEAD): fix SEGV when connect fails.

Initial Comment:
The HEAD netutils SEGVs after a connect() fails when it
calls freeaddrinfo(res) on a null pointer.

eg 
pc09011> gdb -c check_tcp.core check_tcp
GNU gdb 4.18 (FreeBSD)
Copyright 1998 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public
License, and you are
welcome to change it and/or distribute copies of it
under certain conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB.  Type "show
warranty" for details.
This GDB was configured as
"i386-unknown-freebsd"...Deprecated bfd_read called at
/usr/src/gnu/usr.bin/binutils/gdb/../../../../contrib/gdb/gdb/dbxread.c
line 2627 in elfstab_build_psymtabs
Deprecated bfd_read called at
/usr/src/gnu/usr.bin/binutils/gdb/../../../../contrib/gdb/gdb/dbxread.c
line 933 in fill_symbuf

Core was generated by `check_tcp'.
Program terminated with signal 11, Segmentation fault.
Reading symbols from /usr/lib/libssl.so.3...done.
Reading symbols from /usr/lib/libcrypto.so.3...done.
Reading symbols from /usr/lib/libc.so.4...done.
Reading symbols from /usr/libexec/ld-elf.so.1...done.
#0  0x281e54bc in freeaddrinfo () from /usr/lib/libc.so.4
(gdb) bt
#0  0x281e54bc in freeaddrinfo () from /usr/lib/libc.so.4
#1  0x804afb8 in my_connect (host_name=0xbfbffc9d
"xena", port=993, sd=0x8053290, proto=6) at netutils.c:263
#2  0x804ae66 in my_tcp_connect (host_name=0xbfbffc9d
"xena", port=993, sd=0x8053290) at netutils.c:193
#3  0x804a8d2 in connect_SSL () at check_tcp.c:561
#4  0x8049e1b in main (argc=3, argv=0xbfbffba4) at
check_tcp.c:241
(gdb) 

Here is a patch.

Unless you tell me otherwise, I will commit it.

pc09011> diff -u netutils.c.orig netutils.c
--- netutils.c.orig     Tue Nov 30 13:15:18 2004
+++ netutils.c  Tue Nov 30 13:13:21 2004
@@ -213,7 +213,7 @@
 my_connect (const char *host_name, int port, int *sd,
int proto)
 {
        struct addrinfo hints;
-       struct addrinfo *res;
+       struct addrinfo *res, *res0;
        char port_str[6];
        int result;
 
@@ -223,13 +223,14 @@
        hints.ai_socktype = (proto == IPPROTO_UDP) ?
SOCK_DGRAM : SOCK_STREAM;
 
        snprintf (port_str, sizeof (port_str), "%d", port);
-       result = getaddrinfo (host_name, port_str,
&hints, &res);
+       result = getaddrinfo (host_name, port_str,
&hints, &res0);
 
        if (result != 0) {
                printf ("%s\n", gai_strerror (result));
                return STATE_UNKNOWN;
        }
        else {
+               res = res0;
                while (res) {
                        /* attempt to create a socket */
                        *sd = socket (res->ai_family,
(proto == IPPROTO_UDP) ?
@@ -260,7 +261,7 @@
                        close (*sd);
                        res = res->ai_next;
                }
-               freeaddrinfo (res);
+               freeaddrinfo (res0);
        }
 
        if (result == 0)

The problem is that the while (res) loop is _not_
exited after a connection failure but updates res with
res->ai_next  ((a NULL pointer eventually) and then
goes on to free it.

On this system (FreeBSD 4.10), errno is 61
(ECONNREFUSED) after the connection failure, ais it
should be but the break in the switch statement doesn't
cause the exit of the while loop before res is updated.

Here is a single step through it
(gdb) s
220             memset (&hints, 0, sizeof (hints));
(gdb) 
221             hints.ai_family = address_family;
(gdb) 
222             hints.ai_protocol = proto;
(gdb) 
223             hints.ai_socktype = (proto ==
IPPROTO_UDP) ? SOCK_DGRAM : SOCK_STREAM;
(gdb) 
225             snprintf (port_str, sizeof (port_str),
"%d", port);
(gdb) 
226             result = getaddrinfo (host_name,
port_str, &hints, &res);
(gdb) 
228             if (result != 0) {
(gdb) 
233                     while (res) {
(gdb) 
235                             *sd = socket
(res->ai_family, (proto == IPPROTO_UDP) ?
(gdb) 
238                             if (*sd < 0) {
(gdb) 
245                             result = connect (*sd,
res->ai_addr, res->ai_addrlen);
(gdb) 
247                             if (result == 0) {
(gdb) 
252                             if (result < 0) {
(gdb) 
253                                     switch (errno) {
(gdb) 
255                                            
was_refused = TRUE;
(gdb) 
260                             close (*sd);
(gdb) 
261                             res = res->ai_next;
(gdb) q
The program is running.  Exit anyway? (y or n) y
pc09011> 


To sum up, I am not convinced that the code is broken
(maybe a better way is adding another break near the
switch statement.

Your comments are welcome.









----------------------------------------------------------------------

>Comment By: Stanley Hopcroft (stanleyhopcroft)
Date: 2004-12-06 00:45

Message:
Logged In: YES 
user_id=395628

Patch applied to CVS.

Works Ok with FreeBSD and Linux 2.2.



----------------------------------------------------------------------

Comment By: Benoit Mortier (opensides)
Date: 2004-12-06 00:20

Message:
Logged In: YES 
user_id=388184

hi, 
 
can you attach your patch to this report thanks, i will look at it 
 
Bye 

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=397599&aid=1075725&group_id=29880