On Tue, Sep 13, 2022 at 11:51:30AM +0200, Martijn van Duren wrote:
> When a connection is reset while we still have an outstanding request,
> the connection from the request to the rest of the structure is removed,
> so we don't send any old data over the new connection.
>
> However, the current code dereferences axc at a couple of places before
> we check it for NULL.
>
> Found the hard way by Mischa Peters while stress testing agentx support
> for vmd.
>
> OK?
ok tb
>
> martijn@
>
> Index: agentx.c
> ===================================================================
> RCS file: /cvs/src/lib/libagentx/agentx.c,v
> retrieving revision 1.16
> diff -u -p -r1.16 agentx.c
> --- agentx.c 29 Aug 2022 12:17:24 -0000 1.16
> +++ agentx.c 13 Sep 2022 09:51:02 -0000
> @@ -2575,8 +2575,8 @@ static void
> agentx_get_finalize(struct agentx_get *axg)
> {
> struct agentx_context *axc = axg->axg_axc;
> - struct agentx_session *axs = axc->axc_axs;
> - struct agentx *ax = axs->axs_ax;
> + struct agentx_session *axs;
> + struct agentx *ax;
> size_t i, j, nvarbind = 0;
> uint16_t error = 0, index = 0;
> struct ax_varbind *vbl;
> @@ -2591,11 +2591,14 @@ agentx_get_finalize(struct agentx_get *a
> }
> }
>
> - if (axg->axg_axc == NULL) {
> + if (axc == NULL) {
> agentx_get_free(axg);
> return;
> }
>
> + axs = axc->axc_axs;
> + ax = axs->axs_ax;
> +
> if ((vbl = calloc(nvarbind, sizeof(*vbl))) == NULL) {
> agentx_log_axg_warn(axg, "Couldn't parse request");
> agentx_get_free(axg);
> @@ -2655,12 +2658,14 @@ agentx_get_free(struct agentx_get *axg)
> {
> struct agentx_varbind *axv;
> struct agentx_object *axo;
> - struct agentx *ax = axg->axg_axc->axc_axs->axs_ax;
> + struct agentx *ax;
> struct agentx_varbind_index *index;
> size_t i, j;
>
> - if (axg->axg_axc != NULL)
> + if (axg->axg_axc != NULL) {
> + ax = axg->axg_axc->axc_axs->axs_ax;
> TAILQ_REMOVE(&(ax->ax_getreqs), axg, axg_ax_getreqs);
> + }
>
> for (i = 0; i < axg->axg_nvarbind; i++) {
> axv = &(axg->axg_varbind[i]);
> @@ -2701,6 +2706,11 @@ agentx_varbind_start(struct agentx_varbi
> agentx_log_axg_fatalx(axv->axv_axg,
> "%s: axv_initialized not set", __func__);
> #endif
> +
> + if (axc == NULL) {
> + agentx_varbind_error_type(axv, AX_PDU_ERROR_PROCESSINGERROR, 1);
> + return;
> + }
>
> bcopy(&(axv->axv_vb.avb_oid), &(axo_search.axo_oid),
> sizeof(axo_search.axo_oid));
>
