On Wed, 2010-01-06 at 17:16 +0100, Bart Van Assche wrote:
> On Wed, Jan 6, 2010 at 5:06 PM, Magnus Fromreide <magfr@...> wrote:
> > On Wed, 2010-01-06 at 14:14 +0000, bvassche@... wrote:
> >> Revision: 17968
> >> http://net-snmp.svn.sourceforge.net/net-snmp/?rev=17968&view=rev
> >> Author: bvassche
> >> Date: 2010-01-06 14:14:07 +0000 (Wed, 06 Jan 2010)
> >>
> >> Log Message:
> >> -----------
> >> Duplicated the second argument of REGISTER_SYSOR_TABLE() / REGISTER_SYSOR_ENTRY()
> >> since that argument is freed while unregistering such an entry. Not sure whether
> >> this could actually trigger a call of free() with a constant string as argument
> >> and I'm neither sure about whether or not this could have triggered a crash.
> >
> > This sounds ridiculous!
> >
> > REGISTER_SYSOR_* are thin wrappers around snmp_call_callbacks and thus
> > each argument is passed to any number of functions.
> >
> > This patch makes a single strdup of one of the arguments.
> >
> > Which one of the (possibly multiple, possibly zero, possibly user
> > provided) handlers is responsible for freeing that value?
> >
> > In the case of snmp_call_callbacks there really can't be any ownership
> > transfer of the arguments.
>
> REGISTER_SYSOR_TABLE() and REGISTER_SYSOR_ENTRY() copy the description
> pointer to sysORTable::OR_descr.
Yes. To a temporary sysORTable instance on the stack.
> And this is what happens during
> unregistration of such an entry:
> * unregister_sysORTable_sess() gets invoked (see also agent_sysORTable.c).
Yes.
> * unregister_sysORTable_sess() calls erase().
Yes, on erase.
> * erase() calls free(entry->data.OR_descr).
Yes.
The problem is that the sysORTable instance in entry->data (and all of
entry) is allocated in register_sysORTable_sess, for the description the
relevant strdup is at line 77 of agent_sysORTable.c.
/MF
|
