From a0ca0871837f56b66d3d566f9c23bae68e366a48 Mon Sep 17 00:00:00 2001 From: Robert Watson Date: Wed, 24 Sep 2008 11:07:03 +0000 Subject: [PATCH] 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 MFC after: 3 days --- sys/netinet/tcp_input.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/sys/netinet/tcp_input.c b/sys/netinet/tcp_input.c index 0cb24a7f3fb6..ab18c6038cab 100644 --- a/sys/netinet/tcp_input.c +++ b/sys/netinet/tcp_input.c @@ -2472,12 +2472,19 @@ dropafterack: 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: