On Thu, May 10, 2012 at 09:38:39PM +0200, Henning Brauer wrote:
> I'm looking for oks on this diff to commit it.

I think this is not correct.

> > @@ -6951,12 +6953,12 @@ done:
> >             struct pf_rule_item     *ri;
> > 
> >             if (pd.pflog & PF_LOG_FORCE || r->log & PF_LOG_ALL)
> > -                   PFLOG_PACKET(&pd, reason, r, a, ruleset);
> > +                   PFLOG_PACKET(&pd, reason, action, r, a, ruleset);
> >             if (s) {
> >                     SLIST_FOREACH(ri, &s->match_rules, entry)
> >                             if (ri->r->log & PF_LOG_ALL)
> > -                                   PFLOG_PACKET(&pd, reason, ri->r, a,
> > -                                       ruleset);
> > +                                   PFLOG_PACKET(&pd, reason, action,
> > +                                       ri->r, a, ruleset);
> >             }
> >     }

The variable action does not hold the final action at this place.
It could be PF_SYNPROXY_DROP PF_DEFER PF_DIVERT PF_AFRT, we don't
want to log this.  It is rewrittren in the switch (action) block
below.

Moving the logging after the switch is not an option as the mbuf
*m0 might get freed there.

We could do PFLOG_PACKET(&pd, reason, action == PF_PASS ?  PF_PASS
: PF_DROP, ri->r, a, ruleset) for most cases.  Unfortunately PF_AFRT
could be set to PF_DROP in some cases.

Any ideas for a better fix?

bluhm

Reply via email to