From: Martijn van Duren <openbsd+tech@list.imperialat.at>
Subject: Re: snmpd/agentx compatibility issue
To: Denis Fondras <openbsd@ledeuns.net>
Cc: tech@openbsd.org
Date: Mon, 18 Mar 2024 10:17:57 +0100

On Sun, 2024-03-17 at 09:31 +0100, Denis Fondras wrote:
> Le Thu, Mar 14, 2024 at 10:39:19AM +0100, Martijn van Duren a écrit :
> > I've played around a bit with the libagentx bits and there's plenty to
> > improve (it's almost as if one gets new insights with experience). But I
> > reckon this is the most straightforward diff to fix the packetid issue.
> > 
> > OK?
> > 
> > martijn@
> > 
> > diff db9d9b4b3c06c791a4f448a5521c340160f4705e 3f7b601986d696996ff30f574183096979f23ec7
> > commit - db9d9b4b3c06c791a4f448a5521c340160f4705e
> > commit + 3f7b601986d696996ff30f574183096979f23ec7
> > blob - 191892d6687fb364b54ff66dd5d572ba8346c77a
> > blob + ad51662c9ca9cb82afa9cf5c532c4d9c141ebc4b
> > --- agentx.c
> > +++ agentx.c
> > @@ -187,7 +187,7 @@ static void agentx_varbind_nosuchinstance(struct agent
> >   static void agentx_varbind_endofmibview(struct agentx_varbind *);
> >   static void agentx_varbind_error_type(struct agentx_varbind *,
> >       enum ax_pdu_error, int);
> > -static int agentx_request(struct agentx *, uint32_t,
> > +static struct agentx_request *agentx_request(struct agentx *,
> >       int (*)(struct ax_pdu *, void *), void *);
> >   static int agentx_request_cmp(struct agentx_request *,
> >       struct agentx_request *);
> > @@ -447,7 +447,7 @@ static int
> >   agentx_session_start(struct agentx_session *axs)
> >   {
> >   	struct agentx *ax = axs->axs_ax;
> > -	uint32_t packetid;
> > +	struct agentx_request *axr;
> >   
> >   #ifdef AX_DEBUG
> >   	if (ax->ax_cstate != AX_CSTATE_OPEN ||
> > @@ -456,18 +456,20 @@ agentx_session_start(struct agentx_session *axs)
> >   		agentx_log_ax_fatalx(ax, "%s: unexpected session open",
> >   		    __func__);
> >   #endif
> > -	packetid = ax_open(ax->ax_ax, axs->axs_timeout, &(axs->axs_oid),
> > -	    &(axs->axs_descr));
> > -	if (packetid == 0) {
> > +	if ((axr = agentx_request(ax, agentx_session_finalize, axs)) == NULL)
> > +		return -1;
> > +	if (ax_open(ax->ax_ax, axs->axs_timeout, axr->axr_packetid,
> > +	    &(axs->axs_oid), &(axs->axs_descr)) == -1) {
> >   		agentx_log_ax_warn(ax, "couldn't generate %s",
> >   		    ax_pdutype2string(AX_PDU_TYPE_OPEN));
> >   		agentx_reset(ax);
> 
> Should not you free() axr here ?

Thanks for having a look.
axr is already freed inside agentx_reset(), so there shouldn't be a
problem here.

martijn@