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

Le Mon, Mar 18, 2024 at 10:17:57AM +0100, Martijn van Duren a écrit :
> 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.
> 

Ok, sorry for missing that :)

> martijn@
>