svn commit: r312049 - stable/11/usr.sbin/bsnmpd/tools/bsnmptools

[Ngie Cooper]

Author: ngie
Date: Fri Jan 13 08:59:08 2017
New Revision: 312049
URL: https://svnweb.freebsd.org/changeset/base/312049

Log:
  MFC r310892,r310894,r310989:
  
  r310892:
  
  Don't call snmp_pdu_free(..) until finished with the pdu and when ready to
  allocate a new one via snmp_pdu_create(..)
  
  This fixes bsnmpwalk, so it no longer crashes after r310729
  
  r310894:
  
  snmp_pdu_free the right object at the right time in snmptool_walk
  
  r310892 was on the right track, but unfortunately it was resolving
  the problem incorrectly and accidentally leaking memory in the
  process.
  
  - Call snmp_pdu_free on req before calling snmp_pdu_create on it
    at the bottom of the outer while loop
  - Call snmp_pdu_free on resp after calling snmpwalk_nextpdu_create
    in the inner loop
  
  r310989:
  
  Call snmp_pdu_free on req/resp with a consistent, correct pattern
  
  - snmp_pdu_free should be called before snmp_pdu_create is called
    again
  - snmp_pdu_free should be called on the resp to snmp_dialog when
    successful
  
  Tested with the following bsnmp commands:
  
    % export SNMPUSER=bsnmp SNMPPASSWD=bsnmptest
    % SNMP_ARGS="-A proto=sha -C context='' -K -P proto=des -v 3 -r 0"
    % bsnmpset $SNMP_ARGS sysLocation="MyAgent"
    % bsnmpget $SNMP_ARGS sysLocation
    % bsnmpwalk $SNMP_ARGS

Modified:
  stable/11/usr.sbin/bsnmpd/tools/bsnmptools/bsnmpget.c
Directory Properties:
  stable/11/   (props changed)

Modified: stable/11/usr.sbin/bsnmpd/tools/bsnmptools/bsnmpget.c
==============================================================================
--- stable/11/usr.sbin/bsnmpd/tools/bsnmptools/bsnmpget.c	Fri Jan 13 08:55:41 2017	(r312048)
+++ stable/11/usr.sbin/bsnmpd/tools/bsnmptools/bsnmpget.c	Fri Jan 13 08:59:08 2017	(r312049)
@@ -400,13 +400,16 @@ snmptool_get(struct snmp_toolinfo *snmpt
 
 		if (snmp_parse_resp(&resp, &req) >= 0) {
 			snmp_output_resp(snmptoolctx, &resp, NULL);
+			snmp_pdu_free(&resp);
 			break;
 		}
 
 		snmp_output_err_resp(snmptoolctx, &resp);
 		if (GET_PDUTYPE(snmptoolctx) == SNMP_PDU_GETBULK ||
-		    !ISSET_RETRY(snmptoolctx))
+		    !ISSET_RETRY(snmptoolctx)) {
+			snmp_pdu_free(&resp);
 			break;
+		}
 
 		/*
 		 * Loop through the object list and set object->error to the
@@ -414,15 +417,17 @@ snmptool_get(struct snmp_toolinfo *snmpt
 		 */
 		if (snmp_object_seterror(snmptoolctx,
 		    &(resp.bindings[resp.error_index - 1]),
-		    resp.error_status) <= 0)
+		    resp.error_status) <= 0) {
+			snmp_pdu_free(&resp);
 			break;
+		}
 
 		fprintf(stderr, "Retrying...\n");
 		snmp_pdu_free(&resp);
 		snmp_pdu_create(&req, GET_PDUTYPE(snmptoolctx));
 	}
 
-	snmp_pdu_free(&resp);
+	snmp_pdu_free(&req);
 
 	return (0);
 }
@@ -498,27 +503,29 @@ snmptool_walk(struct snmp_toolinfo *snmp
 			}
 
 			outputs += rc;
-			snmp_pdu_free(&resp);
 
-			if ((u_int)rc < resp.nbindings)
+			if ((u_int)rc < resp.nbindings) {
+				snmp_pdu_free(&resp);
 				break;
+			}
 
 			snmpwalk_nextpdu_create(op,
 			    &(resp.bindings[resp.nbindings - 1].var), &req);
 			if (op == SNMP_PDU_GETBULK)
 				snmpget_fix_getbulk(&req, GET_MAXREP(snmptoolctx),
 				    GET_NONREP(snmptoolctx));
+			snmp_pdu_free(&resp);
 		}
 
 		/* Just in case our root was a leaf. */
 		if (outputs == 0) {
 			snmpwalk_nextpdu_create(SNMP_PDU_GET, &root, &req);
 			if (snmp_dialog(&req, &resp) == SNMP_CODE_OK) {
-				if (snmp_parse_resp(&resp,&req) < 0)
+				if (snmp_parse_resp(&resp, &req) < 0)
 					snmp_output_err_resp(snmptoolctx, &resp);
 				else
-					snmp_output_resp(snmptoolctx, &(resp), NULL);
-
+					snmp_output_resp(snmptoolctx, &resp,
+					    NULL);
 				snmp_pdu_free(&resp);
 			} else
 				warn("Snmp dialog");
@@ -529,9 +536,12 @@ snmptool_walk(struct snmp_toolinfo *snmp
 			break;
 		}
 
+		snmp_pdu_free(&req);
 		snmp_pdu_create(&req, op);
 	}
 
+	snmp_pdu_free(&req);
+
 	if (rc == 0)
 		return (0);
 	else
@@ -1089,25 +1099,29 @@ snmptool_set(struct snmp_toolinfo *snmpt
 		if (snmp_pdu_check(&req, &resp) > 0) {
 			if (GET_OUTPUT(snmptoolctx) != OUTPUT_QUIET)
 				snmp_output_resp(snmptoolctx, &resp, NULL);
+			snmp_pdu_free(&resp);
 			break;
 		}
 
 		snmp_output_err_resp(snmptoolctx, &resp);
-		if (!ISSET_RETRY(snmptoolctx))
+		if (!ISSET_RETRY(snmptoolctx)) {
+			snmp_pdu_free(&resp);
 			break;
+		}
 
 		if (snmp_object_seterror(snmptoolctx,
 		    &(resp.bindings[resp.error_index - 1]),
-		    resp.error_status) <= 0)
+		    resp.error_status) <= 0) {
+			snmp_pdu_free(&resp);
 			break;
+		}
 
 		fprintf(stderr, "Retrying...\n");
 		snmp_pdu_free(&req);
-		snmp_pdu_free(&resp);
 		snmp_pdu_create(&req, SNMP_PDU_SET);
 	}
 
-	snmp_pdu_free(&resp);
+	snmp_pdu_free(&req);
 
 	return (0);
 }