ipfw: LOR/panic with uid rules

[Stefan Ehmann]

On Wednesday 24 September 2008 10:08:13 Robert Watson wrote:
> On Tue, 23 Sep 2008, Robert Watson wrote:
> > On Tue, 23 Sep 2008, Stefan Ehmann wrote:
> >> Also posted about this problem recently in stable at . But got no replies
> >> there. So I tried on a recent CURRENT but the problem persists:
> >>
> >> ipfw rules using uid are causing a deadlock. eg. allow ip from any to
> >> any uid root A simple HTTP fetch triggers this problem nearly instantly.
> >>
> >> For me, this problem existed in 6.x with PREEMPTION enabled. It was
> >> fixed in 7.0. But in RELENG_7 and head it's back. This is a single
> >> processor i386 machine.
> >
> > This is an interesting edge case -- to prevent lookup of an inpcb in the
> > output path, we normally pass the inpcb reference down to the firewall so
> > it can directly access the cred rather than looking it up.  Thus, we
> > don't recurse the global tcbinfo or inpcb locks normally on the transmit
> > path. However, it looks like we have an edge case here where we've freed
> > the inpcb but not yet unlocked the tcbinfo, and since the inpcb is freed
> > we don't pass it down--the firewall code tries to look up the inpcb and
> > improperly recurses the tcbinfo lock, boom.
> >
> > The uid/gid/jail code in ipfw is undesirable for a number of reasons, not
> > least because it's a layering violation.  Historically, layering
> > violations meant slightly awkward and risky recursion, but now they also
> > mean lock recursion, which has more serious consequences.  I'll
> > investigate tomorrow and see what the best solution is -- probably to
> > drop the lock before calling tcp_dropwithreset() on a NULL inpcb, which
> > is a workaround/hack, but I think our hands are forced in this case. 
> > I'll follow up with a patch then.
>
> Here is a possible candidate patch, could you see if it resolves all of the
> issues you reported?  (Possibly I have missed other similar cases as
> well...)

Thanks for the patch. Unfortunately the LORs and the panic still remain.