Your message dated Wed, 7 Mar 2018 21:49:10 +1100
with message-id <20180307104910.ga18...@enc.com.au>
and subject line Re:  libsnmp-dev: File descriptors larger than FD_SETSIZE 
crash the init_snmp() function
has caused the Debian Bug report #754955,
regarding libsnmp-dev: File descriptors larger than FD_SETSIZE crash the 
init_snmp() function
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact ow...@bugs.debian.org
immediately.)


-- 
754955: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=754955
Debian Bug Tracking System
Contact ow...@bugs.debian.org with problems
--- Begin Message ---
Package: libsnmp-dev
Version: 5.7.2.1~dfsg-5
Severity: important
Tags: patch

Dear Maintainer,

I was debugging a sub agent running on a thread started by a larger
application.
If the process used up more file descriptors than FD_SETSIZE before starting
the thread which was running the sub agent, the sub agent crashed on the
init_snmp() function. This should not be a problem since versions over NetSNMP
5.5 can and should use netsnmp_large_fd_set struct to deal with large file
descriptors.

Debugging the issue I've found that the functions used to manipulate the large
file descriptor sets ( netsnmp_large_fd_setfd(),  netsnmp_large_fd_clr(),
netsnmp_large_fd_is_set(),  netsnmp_large_fd_set_resize() ) use the macros
FD_SET, FD_CLR, FD_ISSET. These macros should be size independent, however in
newer versions of libc library they have an inbuilt buffer overflow protection
which tests agains the FD_SETSIZE, when manipulating a file descriptor from the
set.

Also the functions snmp_synch_response_cb() and snmp_sess_synch_response()
still use the standard fd_set struct, wich causes an infinite loop if the
response is expected on a file descriptor larger than FD_SETSIZE. This is
aready fixed in upstream PATCH 3394386, I've just used the fix.

Attached a suggested patch.

-- System Information:
Debian Release: jessie/sid
  APT prefers trusty-updates
  APT policy: (500, 'trusty-updates'), (500, 'trusty-security'), (500,
'trusty'), (100, 'trusty-backports')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 3.13.0-30-generic (SMP w/4 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash

Versions of packages libsnmp-dev depends on:
ii  libc6-dev        2.19-0ubuntu6
ii  libsensors4-dev  1:3.3.4-2ubuntu1
ii  libsnmp30        5.7.2.1~dfsg-5
ii  libssl-dev       1.0.1f-1ubuntu2.4
ii  libwrap0-dev     7.6.q-25
ii  procps           1:3.3.9-1ubuntu2

libsnmp-dev recommends no packages.

libsnmp-dev suggests no packages.
commit 530958fd97c34ebb213f4fb82746af126b00eb14
Author: Petr Zajicek <petr.zaji...@nangu.tv>
Date:   Tue Jul 15 15:20:21 2014 +0200

    Bug 36980: Improved netsnmp_large_fd_set struct support in sub agents.
    
    Changes to be committed:
    	modified:   snmplib/large_fd_set.c
    	modified:   snmplib/snmp_client.c

diff --git a/snmplib/large_fd_set.c b/snmplib/large_fd_set.c
index 32f57b3..ffe37f8 100644
--- a/snmplib/large_fd_set.c
+++ b/snmplib/large_fd_set.c
@@ -79,6 +79,10 @@ netsnmp_large_fd_is_set(SOCKET fd, netsnmp_large_fd_set * fdset)
 
 #else
 
+const unsigned int number_of_bits = (8 * (int) sizeof (__fd_mask));
+inline unsigned int pos_in_array( int fd ) { return fd / number_of_bits; }
+inline __fd_mask get_mask_for_fd( int fd ) { return (__fd_mask) ( 1UL << (fd % number_of_bits) ); }
+
 void
 netsnmp_large_fd_setfd(int fd, netsnmp_large_fd_set * fdset)
 {
@@ -87,7 +91,7 @@ netsnmp_large_fd_setfd(int fd, netsnmp_large_fd_set * fdset)
     while (fd >= (int)fdset->lfs_setsize)
         netsnmp_large_fd_set_resize(fdset, 2 * (fdset->lfs_setsize + 1));
 
-    FD_SET(fd, fdset->lfs_setptr);
+    ((__fd_mask*)(fdset->lfs_setptr))[ pos_in_array( fd ) ] |= get_mask_for_fd( fd );
 }
 
 void
@@ -96,7 +100,9 @@ netsnmp_large_fd_clr(int fd, netsnmp_large_fd_set * fdset)
     netsnmp_assert(fd >= 0);
 
     if ((unsigned)fd < fdset->lfs_setsize)
-        FD_CLR(fd, fdset->lfs_setptr);
+    {
+        ((__fd_mask*)(fdset->lfs_setptr))[ pos_in_array( fd ) ] &= ~get_mask_for_fd( fd );
+    }
 }
 
 int
@@ -104,7 +110,10 @@ netsnmp_large_fd_is_set(int fd, netsnmp_large_fd_set * fdset)
 {
     netsnmp_assert(fd >= 0);
 
-    return (unsigned)fd < fdset->lfs_setsize && FD_ISSET(fd, fdset->lfs_setptr);
+    if( (unsigned)fd > fdset->lfs_setsize )
+        return 0;
+
+    return ((__fd_mask*)(fdset->lfs_setptr))[ pos_in_array( fd ) ] & get_mask_for_fd( fd );
 }
 
 #endif
@@ -182,7 +191,7 @@ netsnmp_large_fd_set_resize(netsnmp_large_fd_set * fdset, int setsize)
          * resized *fdset but that were not defined in the original *fdset.
          */
         for (i = fdset->lfs_setsize; i < setsize; i++)
-            FD_CLR(i, fdset->lfs_setptr);
+            ((__fd_mask*)(fdset->lfs_setptr))[ pos_in_array( i ) ] &= ~get_mask_for_fd( i );
     }
 #endif
 
diff --git a/snmplib/snmp_client.c b/snmplib/snmp_client.c
index c1aa9c4..998776f 100644
--- a/snmplib/snmp_client.c
+++ b/snmplib/snmp_client.c
@@ -97,6 +97,7 @@ SOFTWARE.
 #include <net-snmp/library/mib.h>
 #include <net-snmp/library/snmp_logging.h>
 #include <net-snmp/library/snmp_assert.h>
+#include <net-snmp/library/large_fd_set.h>
 #include <net-snmp/pdu_api.h>
 
 netsnmp_feature_child_of(snmp_client_all, libnetsnmp)
@@ -110,17 +111,6 @@ netsnmp_feature_child_of(row_create, snmp_client_all)
 #define BSD4_2
 #endif
 
-#ifndef FD_SET
-
-typedef long    fd_mask;
-#define NFDBITS	(sizeof(fd_mask) * NBBY)        /* bits per mask */
-
-#define	FD_SET(n, p)	((p)->fds_bits[(n)/NFDBITS] |= (1 << ((n) % NFDBITS)))
-#define	FD_CLR(n, p)	((p)->fds_bits[(n)/NFDBITS] &= ~(1 << ((n) % NFDBITS)))
-#define	FD_ISSET(n, p)	((p)->fds_bits[(n)/NFDBITS] & (1 << ((n) % NFDBITS)))
-#define FD_ZERO(p)	memset((p), 0, sizeof(*(p)))
-#endif
-
 /*
  * Prototype definitions 
  */
@@ -1029,13 +1019,13 @@ snmp_synch_response_cb(netsnmp_session * ss,
                        netsnmp_pdu *pdu,
                        netsnmp_pdu **response, snmp_callback pcb)
 {
-    struct synch_state lstate, *state;
-    snmp_callback   cbsav;
-    void           *cbmagsav;
-    int             numfds, count;
-    fd_set          fdset;
-    struct timeval  timeout, *tvp;
-    int             block;
+    struct synch_state    lstate, *state;
+    snmp_callback         cbsav;
+    void                 *cbmagsav;
+    int                   numfds, count;
+    netsnmp_large_fd_set  fdset;
+    struct timeval        timeout, *tvp;
+    int                   block;
 
     memset((void *) &lstate, 0, sizeof(lstate));
     state = &lstate;
@@ -1043,6 +1033,7 @@ snmp_synch_response_cb(netsnmp_session * ss,
     cbmagsav = ss->callback_magic;
     ss->callback = pcb;
     ss->callback_magic = (void *) state;
+    netsnmp_large_fd_set_init(&fdset, FD_SETSIZE);
 
     if ((state->reqid = snmp_send(ss, pdu)) == 0) {
         snmp_free_pdu(pdu);
@@ -1052,17 +1043,17 @@ snmp_synch_response_cb(netsnmp_session * ss,
 
     while (state->waiting) {
         numfds = 0;
-        FD_ZERO(&fdset);
+        NETSNMP_LARGE_FD_ZERO(&fdset);
         block = NETSNMP_SNMPBLOCK;
         tvp = &timeout;
         timerclear(tvp);
-        snmp_sess_select_info_flags(0, &numfds, &fdset, tvp, &block,
+        snmp_sess_select_info2_flags(0, &numfds, &fdset, tvp, &block,
                                     NETSNMP_SELECT_NOALARMS);
         if (block == 1)
             tvp = NULL;         /* block without timeout */
-        count = select(numfds, &fdset, NULL, NULL, tvp);
+        count = netsnmp_large_fd_set_select(numfds, &fdset, NULL, NULL, tvp);
         if (count > 0) {
-            snmp_read(&fdset);
+            snmp_read2(&fdset);
         } else {
             switch (count) {
             case 0:
@@ -1101,6 +1092,7 @@ snmp_synch_response_cb(netsnmp_session * ss,
     *response = state->pdu;
     ss->callback = cbsav;
     ss->callback_magic = cbmagsav;
+    netsnmp_large_fd_set_cleanup(&fdset);
     return state->status;
 }
 
@@ -1115,14 +1107,14 @@ int
 snmp_sess_synch_response(void *sessp,
                          netsnmp_pdu *pdu, netsnmp_pdu **response)
 {
-    netsnmp_session *ss;
-    struct synch_state lstate, *state;
-    snmp_callback   cbsav;
-    void           *cbmagsav;
-    int             numfds, count;
-    fd_set          fdset;
-    struct timeval  timeout, *tvp;
-    int             block;
+    netsnmp_session      *ss;
+    struct synch_state    lstate, *state;
+    snmp_callback         cbsav;
+    void                 *cbmagsav;
+    int                   numfds, count;
+    netsnmp_large_fd_set  fdset;
+    struct timeval        timeout, *tvp;
+    int                   block;
 
     ss = snmp_sess_session(sessp);
     if (ss == NULL) {
@@ -1135,6 +1127,7 @@ snmp_sess_synch_response(void *sessp,
     cbmagsav = ss->callback_magic;
     ss->callback = snmp_synch_input;
     ss->callback_magic = (void *) state;
+    netsnmp_large_fd_set_init(&fdset, FD_SETSIZE);
 
     if ((state->reqid = snmp_sess_send(sessp, pdu)) == 0) {
         snmp_free_pdu(pdu);
@@ -1144,17 +1137,17 @@ snmp_sess_synch_response(void *sessp,
 
     while (state->waiting) {
         numfds = 0;
-        FD_ZERO(&fdset);
+        NETSNMP_LARGE_FD_ZERO(&fdset);
         block = NETSNMP_SNMPBLOCK;
         tvp = &timeout;
         timerclear(tvp);
-        snmp_sess_select_info_flags(sessp, &numfds, &fdset, tvp, &block,
+        snmp_sess_select_info2_flags(sessp, &numfds, &fdset, tvp, &block,
                                     NETSNMP_SELECT_NOALARMS);
         if (block == 1)
             tvp = NULL;         /* block without timeout */
-        count = select(numfds, &fdset, NULL, NULL, tvp);
+        count = netsnmp_large_fd_set_select(numfds, &fdset, NULL, NULL, tvp);
         if (count > 0) {
-            snmp_sess_read(sessp, &fdset);
+            snmp_sess_read2(sessp, &fdset);
         } else
             switch (count) {
             case 0:
@@ -1185,6 +1178,7 @@ snmp_sess_synch_response(void *sessp,
     *response = state->pdu;
     ss->callback = cbsav;
     ss->callback_magic = cbmagsav;
+    netsnmp_large_fd_set_cleanup(&fdset);
     return state->status;
 }
 

--- End Message ---
--- Begin Message ---
Package: libsnmp-dev
Version: 5.7.4-1

It looks like upstream applied something like your patch in a version
after 5.7.2.1  I'm not sure exactly where this got fixed, but 5.7.4
has it.

 - Craig

-- 
Craig Small               https://dropbear.xyz/     csmall at : enc.com.au
Debian GNU/Linux          https://www.debian.org/   csmall at : debian.org
Mastodon: @smalls...@social.dropbear.xyz               Twitter: @smallsees  
GPG fingerprint:        5D2F B320 B825 D939 04D2  0519 3938 F96B DF50 FEA5

--- End Message ---

Reply via email to