Re: libagentx: don't return responses >= searchrange.end

2023-10-28 Thread Theo Buehler
On Sat, Oct 28, 2023 at 09:41:55AM +0200, Martijn van Duren wrote:
> In most cases when a region is registered we have the full ownership.
> As soon as a region has been registered below prior mentioned region we
> could loose ownership halfway through. This case currently isn't fully
> tested and with indices we can return OIDs >= searchrange.end.
> The easiest way is to test this case is in agentx_varbind_finalize()
> and simply reset a varbind to EOMV if we're outside our range.

Makes sense.

> I've pulled apart the agentx_request_type cases for clarity and control.

Yes, that's more readable.

ok tb

> 
> OK?
> 
> martijn@
> 
> Index: agentx.c
> ===
> RCS file: /cvs/src/lib/libagentx/agentx.c,v
> retrieving revision 1.23
> diff -u -p -r1.23 agentx.c
> --- agentx.c  24 Oct 2023 08:54:52 -  1.23
> +++ agentx.c  28 Oct 2023 07:40:59 -
> @@ -2822,7 +2822,7 @@ getnext:
>   while (axo != NULL && axo->axo_cstate != AX_CSTATE_OPEN)
>   axo = RB_NEXT(axc_objects, &(axc->axc_objects), axo);
>   if (axo == NULL ||
> - ax_oid_cmp(&(axo->axo_oid), &(axv->axv_end)) > 0) {
> + ax_oid_cmp(&(axo->axo_oid), &(axv->axv_end)) >= 0) {
>   agentx_varbind_endofmibview(axv);
>   return;
>   }
> @@ -3349,19 +3349,53 @@ agentx_varbind_finalize(struct agentx_va
>  #endif
>   }
>   }
> - cmp = ax_oid_cmp(&(axv->axv_vb.avb_oid), );
> - if ((agentx_varbind_request(axv) == AGENTX_REQUEST_TYPE_GETNEXT &&
> - cmp >= 0) || cmp > 0) {
> + cmp = ax_oid_cmp(, &(axv->axv_vb.avb_oid));
> + switch (agentx_varbind_request(axv)) {
> + case AGENTX_REQUEST_TYPE_GET:
> + if (cmp != 0) {
>  #ifdef AX_DEBUG
> - agentx_log_axg_fatalx(axg, "indices not incremented");
> + agentx_log_axg_fatalx(axg, "index changed");
>  #else
> - agentx_log_axg_warnx(axg, "indices not incremented");
> - bcopy(&(axv->axv_start), &(axv->axv_vb.avb_oid),
> - sizeof(axv->axv_start));
> - axv->axv_error = AX_PDU_ERROR_GENERR;
> + agentx_log_axg_warnx(axg, "index changed");
> + bcopy(&(axv->axv_start), &(axv->axv_vb.avb_oid),
> + sizeof(axv->axv_start));
> + axv->axv_error = AX_PDU_ERROR_GENERR;
> + break;
>  #endif
> - } else
> + }
> + break;
> + case AGENTX_REQUEST_TYPE_GETNEXT:
> + if (cmp <= 0) {
> +#ifdef AX_DEBUG
> + agentx_log_axg_fatalx(axg, "indices not incremented");
> +#else
> + agentx_log_axg_warnx(axg, "indices not incremented");
> + bcopy(&(axv->axv_start), &(axv->axv_vb.avb_oid),
> + sizeof(axv->axv_start));
> + axv->axv_error = AX_PDU_ERROR_GENERR;
> + break;
> +#endif
> + }
> + /* FALLTHROUGH */
> + case AGENTX_REQUEST_TYPE_GETNEXTINCLUSIVE:
> + if (cmp < 0) {
> +#ifdef AX_DEBUG
> + agentx_log_axg_fatalx(axg, "index decremented");
> +#else
> + agentx_log_axg_warnx(axg, "index decremented");
> + bcopy(&(axv->axv_start), &(axv->axv_vb.avb_oid),
> + sizeof(axv->axv_start));
> + axv->axv_error = AX_PDU_ERROR_GENERR;
> + break;
> +#endif
> + }
> + if (axv->axv_end.aoi_idlen != 0 &&
> + ax_oid_cmp(, &(axv->axv_end)) >= 0) {
> + agentx_varbind_endofmibview(axv);
> + return;
> + }
>   bcopy(, &(axv->axv_vb.avb_oid), sizeof(oid));
> + }
>  done:
>   agentx_object_unlock(axv->axv_axo);
>   agentx_get_finalize(axv->axv_axg);
> 



libagentx: don't return responses >= searchrange.end

2023-10-28 Thread Martijn van Duren
In most cases when a region is registered we have the full ownership.
As soon as a region has been registered below prior mentioned region we
could loose ownership halfway through. This case currently isn't fully
tested and with indices we can return OIDs >= searchrange.end.
The easiest way is to test this case is in agentx_varbind_finalize()
and simply reset a varbind to EOMV if we're outside our range.

I've pulled apart the agentx_request_type cases for clarity and control.

OK?

martijn@

Index: agentx.c
===
RCS file: /cvs/src/lib/libagentx/agentx.c,v
retrieving revision 1.23
diff -u -p -r1.23 agentx.c
--- agentx.c24 Oct 2023 08:54:52 -  1.23
+++ agentx.c28 Oct 2023 07:40:59 -
@@ -2822,7 +2822,7 @@ getnext:
while (axo != NULL && axo->axo_cstate != AX_CSTATE_OPEN)
axo = RB_NEXT(axc_objects, &(axc->axc_objects), axo);
if (axo == NULL ||
-   ax_oid_cmp(&(axo->axo_oid), &(axv->axv_end)) > 0) {
+   ax_oid_cmp(&(axo->axo_oid), &(axv->axv_end)) >= 0) {
agentx_varbind_endofmibview(axv);
return;
}
@@ -3349,19 +3349,53 @@ agentx_varbind_finalize(struct agentx_va
 #endif
}
}
-   cmp = ax_oid_cmp(&(axv->axv_vb.avb_oid), );
-   if ((agentx_varbind_request(axv) == AGENTX_REQUEST_TYPE_GETNEXT &&
-   cmp >= 0) || cmp > 0) {
+   cmp = ax_oid_cmp(, &(axv->axv_vb.avb_oid));
+   switch (agentx_varbind_request(axv)) {
+   case AGENTX_REQUEST_TYPE_GET:
+   if (cmp != 0) {
 #ifdef AX_DEBUG
-   agentx_log_axg_fatalx(axg, "indices not incremented");
+   agentx_log_axg_fatalx(axg, "index changed");
 #else
-   agentx_log_axg_warnx(axg, "indices not incremented");
-   bcopy(&(axv->axv_start), &(axv->axv_vb.avb_oid),
-   sizeof(axv->axv_start));
-   axv->axv_error = AX_PDU_ERROR_GENERR;
+   agentx_log_axg_warnx(axg, "index changed");
+   bcopy(&(axv->axv_start), &(axv->axv_vb.avb_oid),
+   sizeof(axv->axv_start));
+   axv->axv_error = AX_PDU_ERROR_GENERR;
+   break;
 #endif
-   } else
+   }
+   break;
+   case AGENTX_REQUEST_TYPE_GETNEXT:
+   if (cmp <= 0) {
+#ifdef AX_DEBUG
+   agentx_log_axg_fatalx(axg, "indices not incremented");
+#else
+   agentx_log_axg_warnx(axg, "indices not incremented");
+   bcopy(&(axv->axv_start), &(axv->axv_vb.avb_oid),
+   sizeof(axv->axv_start));
+   axv->axv_error = AX_PDU_ERROR_GENERR;
+   break;
+#endif
+   }
+   /* FALLTHROUGH */
+   case AGENTX_REQUEST_TYPE_GETNEXTINCLUSIVE:
+   if (cmp < 0) {
+#ifdef AX_DEBUG
+   agentx_log_axg_fatalx(axg, "index decremented");
+#else
+   agentx_log_axg_warnx(axg, "index decremented");
+   bcopy(&(axv->axv_start), &(axv->axv_vb.avb_oid),
+   sizeof(axv->axv_start));
+   axv->axv_error = AX_PDU_ERROR_GENERR;
+   break;
+#endif
+   }
+   if (axv->axv_end.aoi_idlen != 0 &&
+   ax_oid_cmp(, &(axv->axv_end)) >= 0) {
+   agentx_varbind_endofmibview(axv);
+   return;
+   }
bcopy(, &(axv->axv_vb.avb_oid), sizeof(oid));
+   }
 done:
agentx_object_unlock(axv->axv_axo);
agentx_get_finalize(axv->axv_axg);