On Fri, Nov 24, 2017 at 07:22:58PM +0100, Alexander Bluhm wrote:
> On Fri, Nov 24, 2017 at 01:11:08PM +0100, Alexandr Nedvedicky wrote:
> > the patch below is part of larger diff [1] I've sent earlier. Leonardo
> > seen a
> > pfctl.core, when pfctl_optimize failed to create a radix table. The use
> > after
> > free happens in superblock_free() at 1621:
>
> I have seen exactly the same crash this week. My analysis came to
> the same result as yours. But while I was still considering whether
> a reference count would be overkill for such a short-lived tool,
> you just fixed the bug. Thanks!
thanks for looking at my changes. I had same doubts if I should go
for reference count overkill. Then finally the passion for correct
code won.
>
> > @@ -315,9 +317,10 @@ pfctl_optimize_ruleset(struct pfctl *pf, struct
> > pf_ruleset *rs)
> > err(1, "calloc");
> > memcpy(r, &por->por_rule, sizeof(*r));
> > TAILQ_INSERT_TAIL(rs->rules.active.ptr, r, entries);
> > - free(por);
> > + pf_opt_table_unref(por->por_src_tbl);
> > + pf_opt_table_unref(por->por_dst_tbl);
> > }
> > - free(block);
> > + superblock_free(pf, block);
> > }
> >
> > return (0);
>
> I think you must not remove the free(por) line. It is correct in
> your larger diff, but here you leak memory.
good catch, you are right.
thanks a lot
regards
sasha
