Re: snmpd(8): Allow registering above subtree allocated region

[Theo Buehler]

On Tue, Aug 30, 2022 at 04:47:30PM +0200, Martijn van Duren wrote:
> So right now we disallow allocation of a region if a sub-region has the
> subtree flag set. This logic is inverted compared to it's intention,
> which is that if you allocate a region with subtree it should check that
> the entire region below it is available (application.c:242 should say
> "subtree" instead of "region->subtree").
> 
> However, since the subtree flag is intended for backends that are
> normally only initialized on startup and we now have blocklist, chances
> for overlap increase and could possible disable bigger regions than
> intended.
> 
> I think it's safe to remove this entire overlap check because of the
> following reasoning:
> - First the blocklist is loaded. This should prevent allocating
>   anything underneath it and expects things to be loaded on top of
>   it
> - legacy (internal): This could have regions allocated over it and
>   should have blocklist regions underneath it
> - agentx (as native backend): Same as legacy
> - agentx (dynamic applications): Only get their chance when
>   everything already has been initialised and thus no risk of being
>   inserted below a "subtree" region, because that's being captured
>   by the first overlap check.
> 
> OK?

This makes sense to me

ok tb

snmpd(8): Allow registering above subtree allocated region

[Martijn van Duren]

So right now we disallow allocation of a region if a sub-region has the
subtree flag set. This logic is inverted compared to it's intention,
which is that if you allocate a region with subtree it should check that
the entire region below it is available (application.c:242 should say
"subtree" instead of "region->subtree").

However, since the subtree flag is intended for backends that are
normally only initialized on startup and we now have blocklist, chances
for overlap increase and could possible disable bigger regions than
intended.

I think it's safe to remove this entire overlap check because of the
following reasoning:
- First the blocklist is loaded. This should prevent allocating
  anything underneath it and expects things to be loaded on top of
  it
- legacy (internal): This could have regions allocated over it and
  should have blocklist regions underneath it
- agentx (as native backend): Same as legacy
- agentx (dynamic applications): Only get their chance when
  everything already has been initialised and thus no risk of being
  inserted below a "subtree" region, because that's being captured
  by the first overlap check.

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 -  1.12
+++ application.c   30 Aug 2022 14:47:23 -
@@ -237,14 +237,6 @@ appl_region(struct appl_context *ctx, ui
region->ar_backend != backend)
goto overlap;
 
-   search.ar_oid = *oid;
-   region = RB_NFIND(appl_regions, &(ctx->ac_regions), );
-   if (region != NULL && region->ar_subtree && 
-   region->ar_backend != backend && (
-   appl_region_cmp(, region) == 0 ||
-   appl_region_cmp(, region) == -2))
-   goto overlap;
-
if ((nregion = malloc(sizeof(*nregion))) == NULL) {
log_warn("%s: Can't register %s: Processing error",
backend->ab_name, oidbuf);