Hi,
I've been bothered with my Perl scripts segfaulting due to use of freed data;
it seems that under certain circumstances (e.g., when retries are needed),
you can get callbacks for SNMP reports multiple times, and there is no way
that I can see for the script to know which one is the last one or not.
(Even if it did try to reproduce all the logic, it doesn't have access to the
retry count member.)
It seems to me that the callback should simply never be called for something
like SNMPERR_NOT_IN_TIME_WINDOW; the library should just go about its
business and only call the callback when it actually has a decision (even if
that decision is just “it timed out”, of coures). I've attached a patch that
does exactly this; it seems to fix my segfaults.
I still think it's a bit odd that in the success branches, it suddenly
assumes the magic value is of type struct synch_state* (for good measure,
with a cast-as-lvalue, which I believe is undefined behavior wrt. aliasing);
I haven't done anything about this.
/* Steinar */
--
Homepage: http://www.sesse.net/
Index: net-snmp-5.7.2.1~dfsg/snmplib/snmp_api.c
===================================================================
--- net-snmp-5.7.2.1~dfsg.orig/snmplib/snmp_api.c 2014-10-19 21:22:42.851135962 +0200
+++ net-snmp-5.7.2.1~dfsg/snmplib/snmp_api.c 2014-10-19 21:25:05.988325201 +0200
@@ -5319,67 +5319,59 @@
* should be per session !
*/
- if (callback == NULL
- || callback(NETSNMP_CALLBACK_OP_RECEIVED_MESSAGE, sp,
- pdu->reqid, pdu, magic) == 1) {
- if (pdu->command == SNMP_MSG_REPORT) {
- if (sp->s_snmp_errno == SNMPERR_NOT_IN_TIME_WINDOW ||
- snmpv3_get_report_type(pdu) ==
- SNMPERR_NOT_IN_TIME_WINDOW) {
- /*
- * trigger immediate retry on recoverable Reports
- * * (notInTimeWindow), incr_retries == TRUE to prevent
- * * inifinite resend
- */
- if (rp->retries <= sp->retries) {
- snmp_resend_request(slp, rp, TRUE);
- break;
- } else {
- /* We're done with retries, so no longer waiting for a response */
- ((struct synch_state*)magic)->waiting = 0;
- }
- } else {
- if (SNMPV3_IGNORE_UNAUTH_REPORTS) {
- break;
- } else { /* Set the state to no longer be waiting, since we're done with retries */
- ((struct synch_state*)magic)->waiting = 0;
- }
- }
-
+ if (pdu->command == SNMP_MSG_REPORT) {
+ if (sp->s_snmp_errno == SNMPERR_NOT_IN_TIME_WINDOW ||
+ snmpv3_get_report_type(pdu) ==
+ SNMPERR_NOT_IN_TIME_WINDOW) {
/*
- * Handle engineID discovery.
+ * trigger immediate retry on recoverable Reports
+ * * (notInTimeWindow), incr_retries == TRUE to prevent
+ * * inifinite resend
*/
- if (!sp->securityEngineIDLen && pdu->securityEngineIDLen) {
- sp->securityEngineID =
- (u_char *) malloc(pdu->securityEngineIDLen);
- if (sp->securityEngineID == NULL) {
- /*
- * TODO FIX: recover after message callback *?
- */
+ if (rp->retries <= sp->retries) {
+ snmp_resend_request(slp, rp, TRUE);
+ break;
+ } else {
+ /* We're done with retries, so no longer waiting for a response */
+ ((struct synch_state*)magic)->waiting = 0;
+ }
+ } else {
+ if (SNMPV3_IGNORE_UNAUTH_REPORTS) {
+ break;
+ } else { /* Set the state to no longer be waiting, since we're done with retries */
+ ((struct synch_state*)magic)->waiting = 0;
+ }
+ }
+
+ /*
+ * Handle engineID discovery.
+ */
+ if (!sp->securityEngineIDLen && pdu->securityEngineIDLen) {
+ sp->securityEngineID =
+ (u_char *) malloc(pdu->securityEngineIDLen);
+ if (sp->securityEngineID == NULL) {
+ return -1;
+ }
+ memcpy(sp->securityEngineID, pdu->securityEngineID,
+ pdu->securityEngineIDLen);
+ sp->securityEngineIDLen = pdu->securityEngineIDLen;
+ if (!sp->contextEngineIDLen) {
+ sp->contextEngineID =
+ (u_char *) malloc(pdu->securityEngineIDLen);
+ if (sp->contextEngineID == NULL) {
return -1;
}
- memcpy(sp->securityEngineID, pdu->securityEngineID,
+ memcpy(sp->contextEngineID,
+ pdu->securityEngineID,
pdu->securityEngineIDLen);
- sp->securityEngineIDLen = pdu->securityEngineIDLen;
- if (!sp->contextEngineIDLen) {
- sp->contextEngineID =
- (u_char *) malloc(pdu->
- securityEngineIDLen);
- if (sp->contextEngineID == NULL) {
- /*
- * TODO FIX: recover after message callback *?
- */
- return -1;
- }
- memcpy(sp->contextEngineID,
- pdu->securityEngineID,
- pdu->securityEngineIDLen);
- sp->contextEngineIDLen =
- pdu->securityEngineIDLen;
- }
+ sp->contextEngineIDLen = pdu->securityEngineIDLen;
}
}
+ }
+ if (callback == NULL ||
+ callback(NETSNMP_CALLBACK_OP_RECEIVED_MESSAGE, sp,
+ pdu->reqid, pdu, magic) == 1) {
/*
* Successful, so delete request.
*/
------------------------------------------------------------------------------
Comprehensive Server Monitoring with Site24x7.
Monitor 10 servers for $9/Month.
Get alerted through email, SMS, voice calls or mobile push notifications.
Take corrective actions from your mobile device.
http://p.sf.net/sfu/Zoho
_______________________________________________
Net-snmp-coders mailing list
Net-snmp-coders@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/net-snmp-coders
