Sorry, you are right. I messed up the description of my diff. I just noticed that snmpctl core dumped me. I debugged it to that point and thought it were an easy fix. I didn't investigate further problems with that code.
Thanks, Jan On Sun, May 22, 2016 at 10:04:43PM +1000, Jonathan Matthew wrote: > On Mon, May 09, 2016 at 09:37:08PM +0200, Jan Klemkow wrote: > > Hello, > > > > The function ber_free_elements() sets the variable ber to NULL in most > > error cases and calls ber_free_elements(ber). This causes a NULL > > dereference in ber_free_elements. This patch fix that problem. > > It took me a little while to realise you were talking about > ber_printf_elements here. I'm not overly familiar with this code, but my > reading of it is that callers of ber_free_elements are expected to check for > NULL themselves, and that ber_printf_elements shouldn't try to free anything. > > As you've noticed, when ber_printf_elements jumps to the fail: label, 'ber' > is mostly NULL, because an allocation failed. In the cases where it isn't, > 'ber' points to a previously allocated element that has already been attached > to the element that was passed in as the first argument, so freeing it will > result in use after free later on. It has no way of knowing if the element > passed in is attached to some other element, so freeing that is similarly bad. > > trivial diff will follow when I've tried it out a bit.
