Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[freeBSD] "pass" Bad File Descriptor #8

Closed
liviozanol opened this issue Sep 10, 2019 · 6 comments
Closed

[freeBSD] "pass" Bad File Descriptor #8

liviozanol opened this issue Sep 10, 2019 · 6 comments

Comments

@liviozanol
Copy link

FreeBSD 11.1.
NET-SNMP Version: 5.7.3.

We have some "pass" scripts on snmpd.conf which call a shell script to check some information from ipfw.

The shell script response time is almost "instantaneous".
Calling the specified OID using SNMPGET takes 1 second or more, and eventually timeout ...

Sometimes when the daemon keeps running for a long time, snmpd consumes all CPU and hang.

Using truss "truss -d -D -f -o truss_debug.tmp -p <SNMPD_PID>" I can see a lot of "ERR # 9 'Bad file descriptor' " messages.

I have found this bug https://sourceforge.net/p/net-snmp/bugs/2596/, which is expected to be patched on my net-snmp version.


Partial Truss output log:
"
(...)
31737: 8.760443942 0.000050775 getsockname(8,{ AF_INET 0.0.0.0:161 },0x7fffffffe6b8) = 0 (0x0)
31737: 8.760959163 0.000068585 pipe2(0x7ffffffde9d8,0) = 0 (0x0)
31737: 8.761118820 0.000059085 pipe2(0x7ffffffde9d0,0) = 0 (0x0)
31737: 8.761325342 0.000057131 sigprocmask(SIG_BLOCK,{ SIGHUP|SIGINT|SIGQUIT|SIGILL|SIGTRAP|SIGABRT|SIGEMT|SIGFPE|SIGKILL|SIGBUS|SIGSEGV|SIGSYS|SIGPIPE|SIGALRM|SIGTERM|SIGURG|SIGSTOP|SIGTSTP|SIGCONT|SIGCHLD|SIGTTIN|SIGTTOU|SIGIO|SIGXCPU|SIGXFSZ|SIGVTALRM|SIGPROF|SIGWINCH|SIGINFO|SIGUSR1|SIGUSR2 },{ }) = 0 (0x0)
49714: 8.762116087 0.000000000
31737: 8.762141649 0.000709589 fork() = 49714 (0xc232)
49714: 8.762295510 0.000036108 thr_self(0x804616000) = 0 (0x0)
31737: 8.762325752 0.000043582 sigprocmask(SIG_SETMASK,{ },0x0) = 0 (0x0)
31737: 8.762519911 0.000098616 close(9) = 0 (0x0)
49714: 8.762549943 0.000038203 sysarch(AMD64_SET_FSBASE,0x7ffffffc1358) = 0 (0x0)
49714: 8.762716166 0.000080877 sigprocmask(SIG_SETMASK,{ },0x0) = 0 (0x0)
31737: 8.762768128 0.000109651 close(12) = 0 (0x0)
49714: 8.762878128 0.000029054 close(0) = 0 (0x0)
31737: 8.762903411 0.000037854 close(10) = 0 (0x0)
49714: 8.763153793 0.000065512 dup(0x9) = 0 (0x0)
49714: 8.763300460 0.000054546 close(10) = 0 (0x0)
49714: 8.763464238 0.000053917 close(1) = 0 (0x0)
49714: 8.763605318 0.000049867 dup(0xc) = 1 (0x1)
49714: 8.763740112 0.000048819 close(11) = 0 (0x0)
49714: 8.763864011 0.000046515 close(2) = 0 (0x0)
49714: 8.763985326 0.000046445 dup(0x1) = 2 (0x2)
49714: 8.764134158 0.000047911 getdtablesize() = 1886130 (0x1cc7b2)
49714: 8.764255961 0.000047073 close(1886129) ERR#9 'Bad file descriptor'
49714: 8.764371200 0.000041276 close(1886128) ERR#9 'Bad file descriptor'
49714: 8.764493283 0.000047422 close(1886127) ERR#9 'Bad file descriptor'
49714: 8.764609709 0.000041276 close(1886126) ERR#9 'Bad file descriptor'
49714: 8.764718243 0.000040369 close(1886125) ERR#9 'Bad file descriptor'
49714: 8.764827125 0.000041276 close(1886124) ERR#9 'Bad file descriptor'
49714: 8.764935869 0.000041346 close(1886123) ERR#9 'Bad file descriptor'
49714: 8.765045729 0.000041276 close(1886122) ERR#9 'Bad file descriptor'
49714: 8.765160549 0.000039949 close(1886121) ERR#9 'Bad file descriptor'
49714: 8.765272435 0.000038343 close(1886120) ERR#9 'Bad file descriptor'
49714: 8.765375940 0.000038343 close(1886119) ERR#9 'Bad file descriptor'
49714: 8.765494321 0.000045397 close(1886118) ERR#9 'Bad file descriptor'
(...)
"

@fenner
Copy link
Member

fenner commented Sep 10, 2019

This is due to the code at https://github.com/net-snmp/net-snmp/blob/master/agent/netsnmp_close_fds.c#L35 and the fact that your system supports 1886130 file descriptors - net-snmp is trying to close all of the ones it doesn't need. Does FreeBSD support a way to enumerate the file descriptors a process has open, like Linux's /proc/self/fd?

@liviozanol
Copy link
Author

Maybe you can use fstat -p <pid> or procstat -f <pid>.

fstat Example:
USER CMD PID FD MOUNT INUM MODE SZ|DV R/W
root snmpd 84096 text / 2491114 -rwxr-xr-x 28400 r
root snmpd 84096 wd / 2 drwxr-xr-x 1024 r
root snmpd 84096 root / 2 drwxr-xr-x 1024 r
root snmpd 84096 0 /dev 23 crw-rw-rw- null rw
root snmpd 84096 1 /dev 23 crw-rw-rw- null rw
root snmpd 84096 2 /dev 23 crw-rw-rw- null rw
root snmpd 84096 3 - 1364737 -rw------- 187 w
root snmpd 84096 4 /dev 19 crw-r----- mem r
root snmpd 84096 5 /dev 20 crw-r----- kmem r
root snmpd 84096 6* pipe fffff80010c055f0 <-> fffff80010c05758 0 rw
root snmpd 84096 7* pipe fffff80010c05758 <-> fffff80010c055f0 0 rw
root snmpd 84096 8* internet dgram udp fffff800a1d3f570
root snmpd 84096 11* pipe fffff8001099b2f8 <-> fffff8001099b460 0 rw

procstat Example:
PID COMM FD T V FLAGS REF OFFSET PRO NAME
84096 snmpd text v r r------- - - - /usr/local/sbin/snmpd
84096 snmpd cwd v d r------- - - - /
84096 snmpd root v d r------- - - - /
84096 snmpd 0 v c rw------ 3 0 - /dev/null
84096 snmpd 1 v c rw------ 3 0 - /dev/null
84096 snmpd 2 v c rw------ 3 0 - /dev/null
84096 snmpd 3 v r -w------ 1 105 - -
84096 snmpd 4 v c r------- 1 0 - /dev/mem
84096 snmpd 5 v c r------- 1 0 - /dev/kmem
84096 snmpd 6 p - rw------ 1 0 - -
84096 snmpd 7 p - rw------ 1 0 - -
84096 snmpd 8 s - rw------ 1 0 UDP 0.0.0.0:161 0.0.0.0:0

@liviozanol
Copy link
Author

Well, I also found this:
https://www.freebsd.org/cgi/man.cgi?query=fdescfs&sektion=5&manpath=freebsd-release-ports

So, in the source code, you could change if ((dir = opendir("/proc/self/fd"))) { to if ((dir = opendir("/dev/fd"))) { if the system is freeBSD....

I really don't know how to get the OS using the sources from net-snmp, but found this piece of code the gives me hint: http://net-snmp.sourceforge.net/dev/agent/acconfig_8h_source.html

Or just make it more easy, like this:

(...)
dir = opendir("/proc/self/fd");
if (! dir)
    dir = opendir("/dev/fd");

if (dir) {
(...)

Can I make a pull request?

@bvanassche
Copy link
Contributor

I'd like to keep the Linux and BSD code separate in case /dev/fd would ever be introduced on Linux or /proc/self/fd on BSD. Can you have a look at what I just checked in?

@liviozanol
Copy link
Author

liviozanol commented Sep 16, 2019

I think that will solve the issue. On which version will this be applied?

Just for suggestion, I would make the code a little cleaner, but off course, its up to you.

#ifdef __linux__
    dir = opendir("/proc/self/fd");
#elif defined(__FreeBSD__) || defined(__OpenBSD__)
    dir = opendir("/dev/fd");
#endif

while ((ent = readdir(dir))) {
    if (sscanf(ent->d_name, "%d", &i) == 1) {
        if (i > largest_fd)
            largest_fd = i;
        }
    }
closedir(dir);

if (largest_fd < 0)
        largest_fd = getdtablesize() - 1;
for (i = largest_fd; i > fd && i >= 0; i--)
        close(i);

@bvanassche
Copy link
Contributor

Wouldn't what has been suggested above confuse all source code editors that support automatic indentation?

The patch has been applied on the v5.8 and master branches. The first release in which this patch will be included will have version number v5.8.1.

mtremer pushed a commit to ipfire/ipfire-2.x that referenced this issue Jun 4, 2021
- Update from 5.8 to 5.9.1
- Update rootfile
- find-dependencies run to check impact of so lib bump
   no issues found
- Changelog - for more details on the Many bug fixes for 5.9.1 see the
   ChangeLog file in the source tarball
   The following is from the CHANGES file in the source tarball
   *5.9.1*:
     General: Many bug fixes
   *5.9*
     snmplib:
      - Add IPv6 support to DTLSUDP transport
      - use new netsnmp_sockaddr_storage in netsnmp_addr_pair
      - add base_transport ptr for tunneled transports
      - Add support for OpenSSL 1.1.1
      - Dtls: overhaul of debug
      - Remove inline versions of container funcs
     snmpd:
      - Use ETHTOOL_GLINKSETTINGS when available Newer Linux kernels
	support ETHTOOL_GLINKSETTINGS. Use it when available instead of the
	older and deprecated ETHTOOL_GSET. This patch avoids that the Linux
	kernel reports the following kernel warning: warning: 'snmpd' uses
	legacy ethtool link settings API, link modes are only partially
	reported See also https://sourceforge.net/p/net-snmp/patches/1387/.
	[bvanassche: reworked this patch significantly]
      - Reduce the time needed to execute "pass" scripts on BSD systems See
	also net-snmp/net-snmp#8.
      - [BUG 2926]: Make it possible to set agentXPingInterval for a
	subagent - register agentXPingInterval for the subagent list
	handler, before it was registered for snmp - added agentxTimeout to
	the subagent list handler. It's now possible to set for snmpd and
	the subagent. See 'man snmpd.conf' - added agentxRetries to the
	subagent list handler. See 'man snmpd.conf'. It's never used in the
	subagent, but it's now following the documentation Signed-off-by:
	Anders Wallin <wallinux@gmail.com>
     snmptrap:
      - BUG: 2899: Patch from Drew Roedersheimer to set library
	engineboots/time values before sending
     snmptrapd:
      - Add support for the latest libmysqlclient version
     libsnmp:
      - Scan MIB directories in alphabetical order This guarantees that
	e.g. mibs/RFC1213-MIB.txt is read before mibs/SNMPv2-MIB.txt. The
	order in which these MIBs is read matters because both define
	sysLocation but with different attributes.
     unspecified:
      - [BUG 2930]: Fix a Solaris hrSWInst crash Avoid that snmpd crashes
	on Solaris when querying software packages with an empty CATEGORY
	field. See also https://sourceforge.net/p/net-snmp/bugs/2930/. See
	also https://sourceforge.net/p/net-snmp/patches/1390/.
     FreeBSD:
      - Fix first byte of IF-MIB::ifPhysAddress   Don't write past the
	interface name, and use temporary copy instead. This fixes the
	first byte of ifPhysAddress always being 0 on FreeBSD. See also
	https://sourceforge.net/p/net-snmp/code/merge-requests/20/. [
	bvanassche: edited patch title / added test for malloc() result /
	reduced number of free(if_name) calls ]
     Win32:
      - BUG: 2779541 Fixed handle leak in pass_persist.

Signed-off-by: Adolf Belka <adolf.belka@ipfire.org>
Signed-off-by: Michael Tremer <michael.tremer@ipfire.org>
bvanassche added a commit that referenced this issue Apr 16, 2023
Fix the following address sanitizer complaint:

ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7f4e612001a0 at pc 0x7f4e63c0a654 bp 0x7ffc98f7bb80 sp 0x7ffc98f7bb78
WRITE of size 1 at 0x7f4e612001a0 thread T0
    #0 0x7f4e63c0a653 in get_token snmplib/parse.c:4835:13
    #1 0x7f4e63c0a4a2 in get_token snmplib/parse.c:4813:20
    #2 0x7f4e63c0a4a2 in get_token snmplib/parse.c:4813:20
    #3 0x7f4e63c0a4a2 in get_token snmplib/parse.c:4813:20
    #4 0x7f4e63c0a4a2 in get_token snmplib/parse.c:4813:20
    #5 0x7f4e63c0a4a2 in get_token snmplib/parse.c:4813:20
    #6 0x7f4e63c0a4a2 in get_token snmplib/parse.c:4813:20
    #7 0x7f4e63c2dbd2 in parse_ranges snmplib/parse.c:2282:20
    #8 0x7f4e63c29ffa in parse_asntype snmplib/parse.c:2462:27
    #9 0x7f4e63c1789f in parse snmplib/parse.c:4598:19
    #10 0x7f4e63c0497b in read_module_internal snmplib/parse.c:3942:18
    #11 0x7f4e63c0407b in netsnmp_read_module snmplib/parse.c:4041:14
    #12 0x7f4e63c0d5e1 in read_mib snmplib/parse.c:5044:12
    #13 0x55ec21ab37b1 in main testing/fulltests/unit-tests/T027read_mib_clib.c:35:1

Fixes: 34058af ("*/*: replace horrible (char *)"string" with const specifiers in relevant 	functions.")
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants