On Thu, Nov 26, 2015 at 10:02:37AM +0100, Tim van der Molen wrote:
> Tim van der Molen (2015-11-22 21:06 +0100):
> > David CARLIER (2015-11-21 18:24 +0100):
> > > As a first message, I wanted to send a little patch which is all about
> > > use-after-free if fine with you first in mta???s part when a route might
> > > be totally discarded when disabled, the other changes are queue/tree
> > > element removals rearrangements.
> >
> > [...]
> >
> > That won't work I'm afraid. I think the following diff does, but I'll
> > have to defer this to the OpenSMTPD developers.
> >
> > Index: mta.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/smtpd/mta.c,v
> > retrieving revision 1.192
> > diff -p -u -r1.192 mta.c
> > --- mta.c 14 Oct 2015 22:01:43 -0000 1.192
> > +++ mta.c 22 Nov 2015 19:48:24 -0000
> > @@ -1246,13 +1246,13 @@ mta_route_disable(struct mta_route *rout
> > log_info("smtp-out: Disabling route %s for %llus",
> > mta_route_to_text(route), delay);
> >
> > - if (route->flags & ROUTE_DISABLED) {
> > + if (!(route->flags & ROUTE_DISABLED))
> > + mta_route_ref(route);
> > + else
> > runq_cancel(runq_route, NULL, route);
> > - mta_route_unref(route); /* from last call to here */
> > - }
> > +
> > route->flags |= reason & ROUTE_DISABLED;
> > runq_schedule(runq_route, time(NULL) + delay, NULL, route);
> > - mta_route_ref(route);
> > }
>
> I just committed (a slightly different version of) this diff.
>
> However, eric@ has confirmed there is *no* use after free here, because
> the route is referenced at least twice in this case.
>
I was going to reply the same, but I agree this needs to be rewritten in
a slightly different way, it has confused 4 people so far ...
--
Gilles Chehade
https://www.poolp.org @poolpOrg
--
You received this mail because you are subscribed to [email protected]
To unsubscribe, send a mail to: [email protected]
