clicon / clixon

YANG-based toolchain including NETCONF and RESTCONF interfaces and an interactive CLI
http://www.clicon.org/
Other
215 stars 72 forks source link

clixon_snmp module crashes on snmpwalk command #378

Closed vvsosed closed 2 years ago

vvsosed commented 2 years ago

I use command snmpwalk:

snmpwalk -m ALL -v2c -c private 10.40.0.149
Bad operator (INTEGER): At line 73 in /usr/share/snmp/mibs/ietf/SNMPv2-PDU
SNMPv2-MIB::sysDescr = STRING: MH-N366.
SNMPv2-MIB::sysObjectID = OID: SNMPv2-SMI::enterprises.31926.35
SNMPv2-MIB::sysUpTime = Timeticks: (26700) 0:04:27.00
SNMPv2-MIB::sysName = STRING: 00000000.
SNMPv2-MIB::sysServices = INTEGER: 127
IF-MIB::ifNumber = INTEGER: 3
IF-MIB::ifIndex.3 = INTEGER: 3
IF-MIB::ifIndex.4 = INTEGER: 4
IF-MIB::ifIndex.5 = INTEGER: 5
IF-MIB::ifDescr.3 = STRING: eth1.
IF-MIB::ifDescr.4 = STRING: eth2.
IF-MIB::ifDescr.5 = STRING: eth3.
IF-MIB::ifMtu.3 = INTEGER: 1500
IF-MIB::ifMtu.4 = INTEGER: 1500
IF-MIB::ifMtu.5 = INTEGER: 1500
IF-MIB::ifSpeed.3 = Gauge32: 1000000000
IF-MIB::ifSpeed.5 = Gauge32: 4294967295
IF-MIB::ifPhysAddress.3 = STRING: 0:24:a4:16:4f:2a
IF-MIB::ifPhysAddress.4 = STRING: 0:24:a4:16:4f:29
IF-MIB::ifPhysAddress.5 = STRING: 0:24:a4:16:4f:2b
IF-MIB::ifAdminStatus.3 = INTEGER: up(1)
IF-MIB::ifAdminStatus.4 = INTEGER: up(1)
IF-MIB::ifAdminStatus.5 = INTEGER: up(1)
IF-MIB::ifOperStatus.3 = INTEGER: up(1)
IF-MIB::ifOperStatus.4 = INTEGER: down(2)
IF-MIB::ifOperStatus.5 = INTEGER: down(2)
IF-MIB::ifInOctets.3 = Counter32: 3277
IF-MIB::ifInOctets.4 = Counter32: 0
IF-MIB::ifInOctets.5 = Counter32: 0
IF-MIB::ifInUcastPkts.3 = Counter32: 0
IF-MIB::ifInUcastPkts.4 = Counter32: 0
IF-MIB::ifInUcastPkts.5 = Counter32: 0
IF-MIB::ifInDiscards.3 = Counter32: 3
IF-MIB::ifInDiscards.4 = Counter32: 0
IF-MIB::ifInDiscards.5 = Counter32: 0
IF-MIB::ifInErrors.3 = Counter32: 0
IF-MIB::ifInErrors.4 = Counter32: 0
IF-MIB::ifInErrors.5 = Counter32: 0
IF-MIB::ifOutOctets.3 = Counter32: 356
IF-MIB::ifOutOctets.4 = Counter32: 0
IF-MIB::ifOutOctets.5 = Counter32: 0
IF-MIB::ifOutUcastPkts.3 = Counter32: 0
IF-MIB::ifOutUcastPkts.4 = Counter32: 0
IF-MIB::ifOutUcastPkts.5 = Counter32: 0
IF-MIB::ifOutErrors.3 = Counter32: 0
IF-MIB::ifOutErrors.4 = Counter32: 0
IF-MIB::ifOutErrors.5 = Counter32: 0
IF-MIB::ifHighSpeed.5 = Gauge32: 10000
Error in packet.
Reason: (genError) A general failure occured
Failed object: IF-MIB::ifHighSpeed.5

The crash occurs inside clixon_snmp. The unwinded crash dump is added (for bt and bt full) crash-dump_full.txt crash-dump_gdb_bt.txt

At the same time, outputting of IF-MIB table values with netconf CLI completes successfully:

MH-N366@00000000>show IF-MIB 
IF-MIB:IF-MIB {
    interfaces {
        ifNumber 3;
    }
    ifTable {
        ifEntry 3 {
            ifDescr eth1;
            ifMtu 1500;
            ifSpeed 1000000000;
            ifPhysAddress 00:24:a4:16:4f:2a;
            ifAdminStatus up;
            ifOperStatus up;
            ifInOctets 47384;
            ifInUcastPkts 0;
            ifInDiscards 3;
            ifInErrors 0;
            ifOutOctets 356;
            ifOutUcastPkts 0;
            ifOutErrors 0;
            ifAlias;
        }
        ifEntry 4 {
            ifDescr eth2;
            ifMtu 1500;
            ifPhysAddress 00:24:a4:16:4f:29;
            ifAdminStatus up;
            ifOperStatus down;
            ifInOctets 0;
            ifInUcastPkts 0;
            ifInDiscards 0;
            ifInErrors 0;
            ifOutOctets 0;
            ifOutUcastPkts 0;
            ifOutErrors 0;
            ifAlias;
        }
        ifEntry 5 {
            ifDescr eth3;
            ifMtu 1500;
            ifSpeed 4294967295;
            ifPhysAddress 00:24:a4:16:4f:2b;
            ifAdminStatus up;
            ifOperStatus down;
            ifInOctets 0;
            ifInUcastPkts 0;
            ifInDiscards 0;
            ifInErrors 0;
            ifOutOctets 0;
            ifOutUcastPkts 0;
            ifOutErrors 0;
            ifHighSpeed 10000;
            ifAlias;
        }
    }
}
vvsosed commented 2 years ago

Steps to reproduce. I used the attached models. When I add deviation in the "radio-bridge-tg-snmp.yang" file (str. 219-223) the crash appeared. And the contrary if I remove that deviation crash disappears. IANAifType-MIB.yang.txt IF-MIB.yang.txt radio-bridge-tg-snmp.yang.txt SNMPv2-TC.yang.txt Also if I change" config true" to "config false" in node "container IF-MIB" in IF-MIB.yang file the crash not appears. So it appears only if I try to change "config" property in "container IF-MIB" through "deviation" yang statement. Unfortunately, I have no idea now how mentioned "deviation" connected with the crash in clixon_snmp.

olofhagsand commented 2 years ago

Cannot reproduce. Please try this patch:

 diff --git a/apps/snmp/snmp_handler.c b/apps/snmp/snmp_handler.c
index c113214d..750dd04e 100644
--- a/apps/snmp/snmp_handler.c
+++ b/apps/snmp/snmp_handler.c
@@ -146,13 +146,14 @@ snmp_scalar_return(cxobj                      *xs,
     char                  *reason = NULL;
     netsnmp_variable_list *requestvb = request->requestvb;
     int                    ret;
+    char                  *body = NULL;

     /* SMI default value, How is this different from yang defaults?
      */
     if (yang_extension_value(ys, "defval", IETF_YANG_SMIV2_NS, NULL, &defaultval) < 0)
        goto done;
-    if (xs != NULL){
-       if ((ret = type_xml2snmp_pre(xml_body(xs), ys, &xmlstr)) < 0)
+    if (xs != NULL && (body = xml_body(xs)) != NULL){
+       if ((ret = type_xml2snmp_pre(body, ys, &xmlstr)) < 0) // XXX <---
            goto done;
        if (ret == 0){
            if ((ret = netsnmp_request_set_error(request, SNMP_ERR_WRONGVALUE)) != SNMPERR_SUCCESS){
@@ -242,6 +243,7 @@ snmp_scalar_get(clicon_handle               h,
     char   *reason = NULL;
     netsnmp_variable_list *requestvb = request->requestvb;
     cxobj  *xcache = NULL;
+    char   *body = NULL;

     clicon_debug(1, "%s", __FUNCTION__);
     /* Prepare backend call by constructing namespace context */
@@ -270,9 +272,8 @@ snmp_scalar_get(clicon_handle               h,
      * 1. From XML to SNMP string, there is a special case for enumeration, and for default value
      * 2. From SNMP string to SNMP binary value which invloves parsing
      */
-    if (x != NULL){
-       assert(xml_spec(x) == ys);
-       if ((ret = type_xml2snmp_pre(xml_body(x), ys, &xmlstr)) < 0)
+    if (x != NULL && (body = xml_body(x)) != NULL){
+       if ((ret = type_xml2snmp_pre(body, ys, &xmlstr)) < 0)
            goto done;
        if (ret == 0){
            if ((ret = netsnmp_request_set_error(request, SNMP_ERR_WRONGVALUE)) != SNMPERR_SUCCESS){