Doing overlapping regions is hard...

At application.c:1341 we currently assign region->ar_oid to oid.
However, with overlapping regions this can cause a recursion, because
region might be the parent of the previous region that would cause the
oid to traverse back and cause a loop.

This can easily be fixed by changing this line to vb->av_oid, but I also
found that the code doesn't produce the cleanest searchrange end,
adjacent regions might not be taken into account on a single run because
of the else statement on line 1363.

With the following registration it will result in:
blocklist: Registering 1.3.6.1.4.1.30155.1.9.129 context() priority(1) 
timeout(1.50s)
AgentX(556403008/718113859): Registering 1.3.6.1.4.1.30155.1 context() 
priority(1) timeout(5.00s)
AgentX(556403008/718113859): Registering 1.3.6.1.4.1.30155.2 context() 
priority(1) timeout(5.00s)
AgentX(556403008/718113859): Registering 1.3.6.1.4.1.30155.5 context() 
priority(1) timeout(5.00s)
AgentX(556403008/718113859): Registering 1.3.6.1.4.1.30155.6 context() 
priority(1) timeout(5.00s)
...
GetNextRequest{333882979, 0, 0, {{1.3.6.1.4.1.30155.1.9.128.1.24.3:null}}}
AgentX(556403008/718113859): GetNextRequest{1517163039, 0, 0, 
{{1.3.6.1.4.1.30155.1.9.128.1.24.3-1.3.6.1.4.1.30155.1.9.129:null}}}
AgentX(556403008/718113859): Response{1517163039, 0, 0, 
{{1.3.6.1.4.1.30155.1.9.128.1.24.3:endOfMibView}}}
blocklist: GetNextRequest{1339241491, 0, 0, 
{{1.3.6.1.4.1.30155.1.9.129(incl)-1.3.6.1.4.1.30155.1.9.130:null}}}
blocklist: Response{1339241491, 0, 0, 
{{1.3.6.1.4.1.30155.1.9.129:endOfMibView}}}
AgentX(556403008/718113859): GetNextRequest{1545296030, 0, 0, 
{{1.3.6.1.4.1.30155.1.9.130(incl)-1.3.6.1.4.1.30155.2:null}}}
AgentX(556403008/718113859): Response{1545296030, 0, 0, 
{{1.3.6.1.4.1.30155.1.10.1.0:2}}}

to
...
GetNextRequest{2078129483, 0, 0, 
{{1.3.6.1.4.1.30155.1.9.129.1.1.1.192.168.153.0.24:null}}}
blocklist: GetNextRequest{-1870096078, 0, 0, 
{{1.3.6.1.4.1.30155.1.9.129.1.1.1.192.168.153.0.24-1.3.6.1.4.1.30155.1.9.130:null}}}
blocklist: Response{-1870096078, 0, 0, 
{{1.3.6.1.4.1.30155.1.9.129.1.1.1.192.168.153.0.24:endOfMibView}}}
AgentX(3175611302/2074212055): GetNextRequest{-867062125, 0, 0, 
{{1.3.6.1.4.1.30155.1.9.130(incl)-1.3.6.1.4.1.30155.3:null}}}
AgentX(3175611302/2074212055): Response{-867062125, 0, 0, 
{{1.3.6.1.4.1.30155.1.10.1.0:2}}}

OK?

martijn@

Index: application.c
===================================================================
RCS file: /cvs/src/usr.sbin/snmpd/application.c,v
retrieving revision 1.12
diff -u -p -r1.12 application.c
--- application.c       29 Aug 2022 18:05:08 -0000      1.12
+++ application.c       30 Aug 2022 14:29:50 -0000
@@ -1287,7 +1287,7 @@ appl_varbind_backend(struct appl_varbind
        struct appl_request_upstream *ureq = ivb->avi_request_upstream;
        struct appl_region search, *region, *pregion;
        struct appl_varbind *vb = &(ivb->avi_varbind);
-       struct ber_oid oid;
+       struct ber_oid oid, nextsibling;
        int next, cmp;
 
        next = ureq->aru_pdu->be_type == SNMP_C_GETNEXTREQ ||
@@ -1338,33 +1338,41 @@ appl_varbind_backend(struct appl_varbind
        }
        ivb->avi_region = region;
        if (next) {
-               oid = region->ar_oid;
+               oid = vb->av_oid;
+               /*
+                * For the searchrange end we only want contiguous regions.
+                * This means directly connecting, or overlapping with the same
+                * backend.
+                */
                do {
                        pregion = region;
                        region = appl_region_next(ureq->aru_ctx, &oid, pregion);
-                       if (region != NULL &&
-                           appl_region_cmp(region, pregion) > 0)
-                               oid = region->ar_oid;
-                       else
+                       if (region == NULL) {
+                               oid = pregion->ar_oid;
                                ober_oid_nextsibling(&oid);
-               } while (region != NULL &&
-                   region->ar_backend == pregion->ar_backend);
-               if (region == NULL) {
-                       vb->av_oid_end = pregion->ar_oid;
-                       if (pregion->ar_instance &&
-                           vb->av_oid_end.bo_n < BER_MAX_OID_LEN)
-                               vb->av_oid_end.bo_id[vb->av_oid_end.bo_n++] = 0;
-                       else
-                               ober_oid_nextsibling(&(vb->av_oid_end));
-               } else {
-                       if (ober_oid_cmp(&(region->ar_oid),
-                           &(ivb->avi_region->ar_oid)) == 2)
-                               vb->av_oid_end = region->ar_oid;
-                       else {
-                               vb->av_oid_end = ivb->avi_region->ar_oid;
-                               ober_oid_nextsibling(&(vb->av_oid_end));
+                               break;
                        }
-               }
+                       cmp = ober_oid_cmp(&(region->ar_oid), &oid);
+                       if (cmp == 2)
+                               oid = region->ar_oid;
+                       else if (cmp == 1) {
+                               /* Break out if we find a gap */
+                               nextsibling = pregion->ar_oid;
+                               ober_oid_nextsibling(&nextsibling);
+                               if (ober_oid_cmp(&(region->ar_oid),
+                                   &nextsibling) != 0) {
+                                       oid = pregion->ar_oid;
+                                       ober_oid_nextsibling(&oid);
+                                       break;
+                               }
+                               oid = region->ar_oid;
+                       } else if (cmp == -2) {
+                               oid = pregion->ar_oid;
+                               ober_oid_nextsibling(&oid);
+                       } else
+                               fatalx("We can't stop/move back on getnext");
+               } while (region->ar_backend == pregion->ar_backend);
+               vb->av_oid_end = oid;
        }
        return 0;
 

Reply via email to