bsnmpd(8): fix and optimize interface description processing

* correctly prepare a buffer to obtain interface description from a kernel and
  truncate long description instead of dropping it altogether and
  spamming logs;
* skip calling strlen() for each description and each SNMP request
  for MIB-II/ifXTable's ifAlias.
* teach bsnmpd to allocate memory dynamically for interface descriptions
  to decrease memory usage for common case and not to break
  if long description occurs;

PR:			217763
Reviewed by:		harti and others
MFC after:		1 week
Differential Revision:	https://reviews.freebsd.org/D16459
This commit is contained in:
Eugene Grosbein 2018-08-18 10:58:44 +00:00
parent 52809cc35b
commit 1a498d2e68
4 changed files with 69 additions and 8 deletions

View File

@ -439,11 +439,15 @@ mibif_restart_mibII_poll_timer(void)
int int
mib_fetch_ifmib(struct mibif *ifp) mib_fetch_ifmib(struct mibif *ifp)
{ {
static int kmib[2] = { -1, 0 }; /* for sysctl net.ifdescr_maxlen */
int name[6]; int name[6];
size_t kmiblen = nitems(kmib);
size_t len; size_t len;
void *newmib; void *newmib;
struct ifmibdata oldmib = ifp->mib; struct ifmibdata oldmib = ifp->mib;
struct ifreq irr; struct ifreq irr;
unsigned int alias_maxlen = MIBIF_ALIAS_SIZE_MAX;
if (fetch_generic_mib(ifp, &oldmib) == -1) if (fetch_generic_mib(ifp, &oldmib) == -1)
return (-1); return (-1);
@ -515,18 +519,69 @@ mib_fetch_ifmib(struct mibif *ifp)
} }
out: out:
/*
* Find sysctl mib for net.ifdescr_maxlen (one time).
* kmib[0] == -1 at first call to mib_fetch_ifmib().
* Then kmib[0] > 0 if we found sysctl mib for net.ifdescr_maxlen.
* Else, kmib[0] == 0 (unexpected error from a kernel).
*/
if (kmib[0] < 0 &&
sysctlnametomib("net.ifdescr_maxlen", kmib, &kmiblen) < 0) {
kmib[0] = 0;
syslog(LOG_WARNING, "sysctlnametomib net.ifdescr_maxlen: %m");
}
/*
* Fetch net.ifdescr_maxlen value every time to catch up with changes.
*/
len = sizeof(alias_maxlen);
if (kmib[0] > 0 && sysctl(kmib, 2, &alias_maxlen, &len, NULL, 0) < 0) {
/* unexpected error from the kernel, use default value */
alias_maxlen = MIBIF_ALIAS_SIZE_MAX;
syslog(LOG_WARNING, "sysctl net.ifdescr_maxlen: %m");
}
/*
* Kernel limit might be decreased after interfaces got
* their descriptions assigned. Try to obtain them anyway.
*/
if (alias_maxlen == 0)
alias_maxlen = MIBIF_ALIAS_SIZE_MAX;
/*
* Allocate maximum memory for a buffer and later reallocate
* to free extra memory.
*/
if ((ifp->alias = malloc(alias_maxlen)) == NULL) {
syslog(LOG_WARNING, "malloc(%d) failed: %m", (int)alias_maxlen);
goto fin;
}
strlcpy(irr.ifr_name, ifp->name, sizeof(irr.ifr_name)); strlcpy(irr.ifr_name, ifp->name, sizeof(irr.ifr_name));
irr.ifr_buffer.buffer = MIBIF_PRIV(ifp)->alias; irr.ifr_buffer.buffer = ifp->alias;
irr.ifr_buffer.length = sizeof(MIBIF_PRIV(ifp)->alias); irr.ifr_buffer.length = alias_maxlen;
if (ioctl(mib_netsock, SIOCGIFDESCR, &irr) == -1) { if (ioctl(mib_netsock, SIOCGIFDESCR, &irr) == -1) {
MIBIF_PRIV(ifp)->alias[0] = 0; free(ifp->alias);
ifp->alias = NULL;
if (errno != ENOMSG) if (errno != ENOMSG)
syslog(LOG_WARNING, "SIOCGIFDESCR (%s): %m", ifp->name); syslog(LOG_WARNING, "SIOCGIFDESCR (%s): %m", ifp->name);
} else if (irr.ifr_buffer.buffer == NULL) { } else if (irr.ifr_buffer.buffer == NULL) {
MIBIF_PRIV(ifp)->alias[0] = 0; free(ifp->alias);
ifp->alias = NULL;
syslog(LOG_WARNING, "SIOCGIFDESCR (%s): too long (%zu)", syslog(LOG_WARNING, "SIOCGIFDESCR (%s): too long (%zu)",
ifp->name, irr.ifr_buffer.length); ifp->name, irr.ifr_buffer.length);
} else {
ifp->alias_size = strnlen(ifp->alias, alias_maxlen) + 1;
if (ifp->alias_size > MIBIF_ALIAS_SIZE)
ifp->alias_size = MIBIF_ALIAS_SIZE;
if (ifp->alias_size < alias_maxlen)
ifp->alias = realloc(ifp->alias, ifp->alias_size);
} }
fin:
ifp->mibtick = get_ticks(); ifp->mibtick = get_ticks();
return (0); return (0);
} }
@ -706,6 +761,10 @@ mibif_free(struct mibif *ifp)
mibif_reset_hc_timer(); mibif_reset_hc_timer();
} }
if (ifp->alias != NULL) {
free(ifp->alias);
ifp->alias = NULL;
}
free(ifp->private); free(ifp->private);
ifp->private = NULL; ifp->private = NULL;
free(ifp->physaddr); free(ifp->physaddr);

View File

@ -57,8 +57,9 @@
#include "snmp_mibII.h" #include "snmp_mibII.h"
#include "mibII_tree.h" #include "mibII_tree.h"
/* maximum size of the interface alias */ /* maximum size of the interface alias unless overridden with net.ifdescr_maxlen */
#define MIBIF_ALIAS_SIZE (64 + 1) #define MIBIF_ALIAS_SIZE (64 + 1)
#define MIBIF_ALIAS_SIZE_MAX 1024
/* /*
* Interface list and flags. * Interface list and flags.
@ -81,8 +82,6 @@ struct mibif_private {
uint64_t hc_imcasts; uint64_t hc_imcasts;
uint64_t hc_ipackets; uint64_t hc_ipackets;
/* this should be made public */
char alias[MIBIF_ALIAS_SIZE];
}; };
#define MIBIF_PRIV(IFP) ((struct mibif_private *)((IFP)->private)) #define MIBIF_PRIV(IFP) ((struct mibif_private *)((IFP)->private))

View File

@ -528,7 +528,7 @@ op_ifxtable(struct snmp_context *ctx, struct snmp_value *value,
break; break;
case LEAF_ifAlias: case LEAF_ifAlias:
ret = string_get(value, MIBIF_PRIV(ifp)->alias, -1); ret = string_get(value, ifp->alias, ifp->alias_size - 1);
break; break;
case LEAF_ifCounterDiscontinuityTime: case LEAF_ifCounterDiscontinuityTime:

View File

@ -80,6 +80,9 @@ struct mibif {
/* to be set by ifType specific modules. This is ifSpecific. */ /* to be set by ifType specific modules. This is ifSpecific. */
struct asn_oid spec_oid; struct asn_oid spec_oid;
char *alias;
size_t alias_size;
/* private data - don't touch */ /* private data - don't touch */
void *private; void *private;
}; };