lib/44552: hijacked select() with no set fds in fd_set crashes a program

[alnsn%yandex.ru@localhost]

>Number:         44552
>Category:       lib
>Synopsis:       hijacked select() with no set fds in fd_set crashes a program
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat Feb 12 01:15:00 +0000 2011
>Originator:     Alexander Nasonov
>Release:        NetBSD 5.99.45 amd64
>Organization:
home sweet home
>Environment:
NetBSD nebeda.localdomain 5.99.45 NetBSD 5.99.45 (GENERIC) #0: Wed Feb  9 
22:10:05 GMT 2011  
root%nebeda.localdomain@localhost:/home/alnsn/src/netbsd-current/src/sys/arch/amd64/compile/obj/GENERIC
 amd64
>Description:
This line in lib/librumphijack/hijack.c

                pfds[j].events = pfds[j].revents = 0;

can be executed when pfds == NULL.
>How-To-Repeat:
Modify tests/lib/librumphijack and run the test.

Index: tests/lib/librumphijack/h_client.c
===================================================================
RCS file: /cvsroot/src/tests/lib/librumphijack/h_client.c,v
retrieving revision 1.1
diff -u -u -r1.1 h_client.c
--- tests/lib/librumphijack/h_client.c  11 Feb 2011 15:38:14 -0000      1.1
+++ tests/lib/librumphijack/h_client.c  12 Feb 2011 00:54:43 -0000
@@ -65,6 +65,22 @@
                if (FD_ISSET(STDIN_FILENO, &rfds))
                        errx(1, "stdin fileno is still set");
                exit(0);
+       } else if (strcmp(argv[1], "select_allunset") == 0) {
+               fd_set fds;
+               struct timeval tv;
+               int rv;
+
+               tv.tv_sec = 0;
+               tv.tv_usec = 1;
+
+               FD_ZERO(&fds);
+
+               rv = select(100, &fds, &fds, &fds, &tv);
+               if (rv == -1)
+                       err(1, "select");
+               if (rv != 0)
+                       errx(1, "select succesful");
+               exit(0);
        } else {
                return ENOTSUP;
        }
Index: tests/lib/librumphijack/t_asyncio.sh
===================================================================
RCS file: /cvsroot/src/tests/lib/librumphijack/t_asyncio.sh,v
retrieving revision 1.1
diff -u -u -r1.1 t_asyncio.sh
--- tests/lib/librumphijack/t_asyncio.sh        11 Feb 2011 15:38:14 -0000      
1.1
+++ tests/lib/librumphijack/t_asyncio.sh        12 Feb 2011 00:54:43 -0000
@@ -47,7 +47,27 @@
        rump.halt
 }
 
+atf_test_case select_allunset cleanup
+select_allunset_head()
+{
+        atf_set "descr" "select() with no set fds in fd_set should not crash"
+}
+
+select_allunset_body()
+{
+
+       atf_check -s exit:0 ${rumpsrv} ${RUMP_SERVER}
+       atf_check -s exit:0 env LD_PRELOAD=/usr/lib/librumphijack.so \
+           $(atf_get_srcdir)/h_client select_allunset
+}
+
+select_allunset_cleanup()
+{
+       rump.halt
+}
+
 atf_init_test_cases()
 {
        atf_add_test_case select_timeout
+       atf_add_test_case select_allunset
 }

>Fix:
Index: lib/librumphijack/hijack.c
===================================================================
RCS file: /cvsroot/src/lib/librumphijack/hijack.c,v
retrieving revision 1.37
diff -u -u -r1.37 hijack.c
--- lib/librumphijack/hijack.c  11 Feb 2011 14:02:12 -0000      1.37
+++ lib/librumphijack/hijack.c  12 Feb 2011 01:09:01 -0000
@@ -563,7 +563,7 @@
        }
 
        if (realnfds) {
-               pfds = malloc(sizeof(*pfds) * realnfds);
+               pfds = calloc(realnfds, sizeof(*pfds));
                if (!pfds)
                        return -1;
        } else {
@@ -572,7 +572,6 @@
 
        for (i = 0, j = 0; i < nfds; i++) {
                incr = 0;
-               pfds[j].events = pfds[j].revents = 0;
                if (readfds && FD_ISSET(i, readfds)) {
                        pfds[j].fd = i;
                        pfds[j].events |= POLLIN;