Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 29 Jul 2019 12:55:48 +0000 (UTC)
From:      "Andrey V. Elsukov" <ae@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r350413 - head/sys/netpfil/ipfw
Message-ID:  <201907291255.x6TCtmKG024423@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: ae
Date: Mon Jul 29 12:55:48 2019
New Revision: 350413
URL: https://svnweb.freebsd.org/changeset/base/350413

Log:
  Avoid possible lock leaking.
  
  After r343619 ipfw uses own locking for packets flow. PULLUP_LEN() macro
  is used in ipfw_chk() to make m_pullup(). When m_pullup() fails, it just
  returns via `goto pullup_failed`. There are two places where PULLUP_LEN()
  is called with IPFW_PF_RLOCK() held.
  
  Add PULLUP_LEN_LOCKED() macro to use in these places to be able release
  the lock, when m_pullup() fails.
  
  Sponsored by:	Yandex LLC

Modified:
  head/sys/netpfil/ipfw/ip_fw2.c

Modified: head/sys/netpfil/ipfw/ip_fw2.c
==============================================================================
--- head/sys/netpfil/ipfw/ip_fw2.c	Mon Jul 29 10:44:04 2019	(r350412)
+++ head/sys/netpfil/ipfw/ip_fw2.c	Mon Jul 29 12:55:48 2019	(r350413)
@@ -1442,9 +1442,9 @@ ipfw_chk(struct ip_fw_args *args)
  * pointer might become stale after other pullups (but we never use it
  * this way).
  */
-#define PULLUP_TO(_len, p, T)	PULLUP_LEN(_len, p, sizeof(T))
+#define	PULLUP_TO(_len, p, T)	PULLUP_LEN(_len, p, sizeof(T))
 #define	EHLEN	(eh != NULL ? ((char *)ip - (char *)eh) : 0)
-#define PULLUP_LEN(_len, p, T)					\
+#define	_PULLUP_LOCKED(_len, p, T, unlock)			\
 do {								\
 	int x = (_len) + T + EHLEN;				\
 	if (mem) {						\
@@ -1453,12 +1453,18 @@ do {								\
 	} else {						\
 		if (__predict_false((m)->m_len < x)) {		\
 			args->m = m = m_pullup(m, x);		\
-			if (m == NULL)				\
+			if (m == NULL) {			\
+				unlock;				\
 				goto pullup_failed;		\
+			}					\
 		}						\
 		p = mtod(m, char *) + (_len) + EHLEN;		\
 	}							\
 } while (0)
+
+#define	PULLUP_LEN(_len, p, T)	_PULLUP_LOCKED(_len, p, T, )
+#define	PULLUP_LEN_LOCKED(_len, p, T)	\
+    _PULLUP_LOCKED(_len, p, T, IPFW_PF_RUNLOCK(chain))
 /*
  * In case pointers got stale after pullups, update them.
  */
@@ -2310,7 +2316,7 @@ do {								\
 
 			case O_TCPOPTS:
 				if (proto == IPPROTO_TCP && offset == 0 && ulp){
-					PULLUP_LEN(hlen, ulp,
+					PULLUP_LEN_LOCKED(hlen, ulp,
 					    (TCP(ulp)->th_off << 2));
 					match = tcpopts_match(TCP(ulp), cmd);
 				}
@@ -2335,7 +2341,7 @@ do {								\
 					uint16_t mss, *p;
 					int i;
 
-					PULLUP_LEN(hlen, ulp,
+					PULLUP_LEN_LOCKED(hlen, ulp,
 					    (TCP(ulp)->th_off << 2));
 					if ((tcpopts_parse(TCP(ulp), &mss) &
 					    IP_FW_TCPOPT_MSS) == 0)
@@ -3182,6 +3188,7 @@ do {								\
 
 		}	/* end of inner loop, scan opcodes */
 #undef PULLUP_LEN
+#undef PULLUP_LEN_LOCKED
 
 		if (done)
 			break;



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201907291255.x6TCtmKG024423>