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:
eugen 2018-08-18 10:58:44 +00:00
parent dc8f15c81c
commit db47817e6d
4 changed files with 69 additions and 8 deletions

View File

@ -439,11 +439,15 @@ mibif_restart_mibII_poll_timer(void)
int
mib_fetch_ifmib(struct mibif *ifp)
{
static int kmib[2] = { -1, 0 }; /* for sysctl net.ifdescr_maxlen */
int name[6];
size_t kmiblen = nitems(kmib);
size_t len;
void *newmib;
struct ifmibdata oldmib = ifp->mib;
struct ifreq irr;
unsigned int alias_maxlen = MIBIF_ALIAS_SIZE_MAX;
if (fetch_generic_mib(ifp, &oldmib) == -1)
return (-1);
@ -515,18 +519,69 @@ mib_fetch_ifmib(struct mibif *ifp)
}
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));
irr.ifr_buffer.buffer = MIBIF_PRIV(ifp)->alias;
irr.ifr_buffer.length = sizeof(MIBIF_PRIV(ifp)->alias);
irr.ifr_buffer.buffer = ifp->alias;
irr.ifr_buffer.length = alias_maxlen;
if (ioctl(mib_netsock, SIOCGIFDESCR, &irr) == -1) {
MIBIF_PRIV(ifp)->alias[0] = 0;
free(ifp->alias);
ifp->alias = NULL;
if (errno != ENOMSG)
syslog(LOG_WARNING, "SIOCGIFDESCR (%s): %m", ifp->name);
} 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)",
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();
return (0);
}
@ -706,6 +761,10 @@ mibif_free(struct mibif *ifp)
mibif_reset_hc_timer();
}
if (ifp->alias != NULL) {
free(ifp->alias);
ifp->alias = NULL;
}
free(ifp->private);
ifp->private = NULL;
free(ifp->physaddr);

View File

@ -57,8 +57,9 @@
#include "snmp_mibII.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_MAX 1024
/*
* Interface list and flags.
@ -81,8 +82,6 @@ struct mibif_private {
uint64_t hc_imcasts;
uint64_t hc_ipackets;
/* this should be made public */
char alias[MIBIF_ALIAS_SIZE];
};
#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;
case LEAF_ifAlias:
ret = string_get(value, MIBIF_PRIV(ifp)->alias, -1);
ret = string_get(value, ifp->alias, ifp->alias_size - 1);
break;
case LEAF_ifCounterDiscontinuityTime:

View File

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