When dropping a packet and issuing a reset during TCP segment handling,

unconditionally drop the tcbinfo lock (after all, we assert it lines
before), but call tcp_dropwithreset() under both inpcb and inpcbinfo
locks only if we pass in an tcpcb.  Otherwise, if the pointer is NULL,
firewall code may later recurse the global tcbinfo lock trying to look
up an inpcb.

This is an instance where a layering violation leads not only
potentially to code reentrace and recursion, but also to lock
recursion, and was revealed by the conversion to rwlocks because
acquiring a read lock on an rwlock already held with a write lock is
forbidden.  When these locks were mutexes, they simply recursed.

Reported by:	Stefan Ehmann <shoesoft at gmx dot net>
MFC after:	3 days
This commit is contained in:
Robert Watson 2008-09-24 11:07:03 +00:00
parent a8d403e102
commit a0ca087183

View File

@ -2472,12 +2472,19 @@ tcp_do_segment(struct mbuf *m, struct tcphdr *th, struct socket *so,
dropwithreset:
KASSERT(headlocked, ("%s: dropwithreset: head not locked", __func__));
tcp_dropwithreset(m, th, tp, tlen, rstreason);
if (tp != NULL)
/*
* If tp is non-NULL, we call tcp_dropwithreset() holding both inpcb
* and global locks. However, if NULL, we must hold neither as
* firewalls may acquire the global lock in order to look for a
* matching inpcb.
*/
if (tp != NULL) {
tcp_dropwithreset(m, th, tp, tlen, rstreason);
INP_WUNLOCK(tp->t_inpcb);
if (headlocked)
INP_INFO_WUNLOCK(&V_tcbinfo);
}
INP_INFO_WUNLOCK(&V_tcbinfo);
if (tp == NULL)
tcp_dropwithreset(m, th, NULL, tlen, rstreason);
return;
drop: