On Tue, Aug 30, 2022 at 04:31:20PM +0200, Martijn van Duren wrote: > 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?
looks good ok tb > > 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; > >