- Rewrite functions that copyin/out NAT configuration, so that they

  calculate required memory size dynamically.
- Fix races on chain re-lock.
- Introduce new field to ip_fw_chain - generation count. Now utilized
  only in the NAT configuration, but can be utilized wider in ipfw.
- Get rid of NAT_BUF_LEN in ip_fw.h

PR:		kern/143653

2 files changed, 74 insertions, 53 deletions

diff --git a/sys/netinet/ipfw/ip_fw_nat.c b/sys/netinet/ipfw/ip_fw_nat.c
index f5f1374..596975d 100644
--- a/sys/netinet/ipfw/ip_fw_nat.c
+++ b/sys/netinet/ipfw/ip_fw_nat.c
@@ -138,7 +138,7 @@ del_redir_spool_cfg(struct cfg_nat *n, struct redir_chain *head)
 	}
 }
 
-static int
+static void
 add_redir_spool_cfg(char *buf, struct cfg_nat *ptr)
 {
 	struct cfg_redir *r, *ser_r;
@@ -199,7 +199,6 @@ add_redir_spool_cfg(char *buf, struct cfg_nat *ptr)
 		/* And finally hook this redir entry. */
 		LIST_INSERT_HEAD(&ptr->redir_chain, r, _next);
 	}
-	return (1);
 }
 
 static int
@@ -344,20 +343,28 @@ lookup_nat(struct nat_list *l, int nat_id)
 static int
 ipfw_nat_cfg(struct sockopt *sopt)
 {
-	struct cfg_nat *ptr, *ser_n;
+	struct cfg_nat *cfg, *ptr;
 	char *buf;
 	struct ip_fw_chain *chain = &V_layer3_chain;
+	int gencnt, len, error = 0;
 
-	buf = malloc(NAT_BUF_LEN, M_IPFW, M_WAITOK | M_ZERO);
-	sooptcopyin(sopt, buf, NAT_BUF_LEN, sizeof(struct cfg_nat));
-	ser_n = (struct cfg_nat *)buf;
+	len = sopt->sopt_valsize;
+	buf = malloc(len, M_TEMP, M_WAITOK | M_ZERO);
+	if ((error = sooptcopyin(sopt, buf, len, sizeof(struct cfg_nat))) != 0)
+		goto out;
+
+	cfg = (struct cfg_nat *)buf;
+	if (cfg->id < 0) {
+		error = EINVAL;
+		goto out;
+	}
 
-	/* check valid parameter ser_n->id > 0 ? */
 	/*
 	 * Find/create nat rule.
 	 */
 	IPFW_WLOCK(chain);
-	ptr = lookup_nat(&chain->nat, ser_n->id);
+	gencnt = chain->gencnt;
+	ptr = lookup_nat(&chain->nat, cfg->id);
 	if (ptr == NULL) {
 		IPFW_WUNLOCK(chain);
 		/* New rule: allocate and init new instance. */
@@ -365,27 +372,27 @@ ipfw_nat_cfg(struct sockopt *sopt)
 		ptr->lib = LibAliasInit(NULL);
 		LIST_INIT(&ptr->redir_chain);
 	} else {
-		/* Entry already present: temporarly unhook it. */
+		/* Entry already present: temporarily unhook it. */
 		LIST_REMOVE(ptr, _next);
-		flush_nat_ptrs(chain, ser_n->id);
+		flush_nat_ptrs(chain, cfg->id);
 		IPFW_WUNLOCK(chain);
 	}
 
 	/*
 	 * Basic nat configuration.
 	 */
-	ptr->id = ser_n->id;
+	ptr->id = cfg->id;
 	/*
 	 * XXX - what if this rule doesn't nat any ip and just
 	 * redirect?
 	 * do we set aliasaddress to 0.0.0.0?
 	 */
-	ptr->ip = ser_n->ip;
-	ptr->redir_cnt = ser_n->redir_cnt;
-	ptr->mode = ser_n->mode;
-	LibAliasSetMode(ptr->lib, ser_n->mode, ser_n->mode);
+	ptr->ip = cfg->ip;
+	ptr->redir_cnt = cfg->redir_cnt;
+	ptr->mode = cfg->mode;
+	LibAliasSetMode(ptr->lib, cfg->mode, cfg->mode);
 	LibAliasSetAddress(ptr->lib, ptr->ip);
-	memcpy(ptr->if_name, ser_n->if_name, IF_NAMESIZE);
+	memcpy(ptr->if_name, cfg->if_name, IF_NAMESIZE);
 
 	/*
 	 * Redir and LSNAT configuration.
@@ -394,15 +401,19 @@ ipfw_nat_cfg(struct sockopt *sopt)
 	del_redir_spool_cfg(ptr, &ptr->redir_chain);
 	/* Add new entries. */
 	add_redir_spool_cfg(&buf[(sizeof(struct cfg_nat))], ptr);
-	free(buf, M_IPFW);
+
 	IPFW_WLOCK(chain);
-	/*
-	 * XXXGL race here: another ipfw_nat_cfg() may already inserted
-	 * entry with the same ser_n->id.
-	 */
+	/* Extra check to avoid race with another ipfw_nat_cfg() */
+	if (gencnt != chain->gencnt &&
+	    ((cfg = lookup_nat(&chain->nat, ptr->id)) != NULL))
+		LIST_REMOVE(cfg, _next);
 	LIST_INSERT_HEAD(&chain->nat, ptr, _next);
+	chain->gencnt++;
 	IPFW_WUNLOCK(chain);
-	return (0);
+
+out:
+	free(buf, M_TEMP);
+	return (error);
 }
 
 static int
@@ -432,52 +443,61 @@ ipfw_nat_del(struct sockopt *sopt)
 static int
 ipfw_nat_get_cfg(struct sockopt *sopt)
 {
-	uint8_t *data;
+	struct ip_fw_chain *chain = &V_layer3_chain;
 	struct cfg_nat *n;
 	struct cfg_redir *r;
 	struct cfg_spool *s;
-	int nat_cnt, off;
-	struct ip_fw_chain *chain;
-	int err = ENOSPC;
+	char *data;
+	int gencnt, nat_cnt, len, error;
 
-	chain = &V_layer3_chain;
 	nat_cnt = 0;
-	off = sizeof(nat_cnt);
+	len = sizeof(nat_cnt);
 
-	data = malloc(NAT_BUF_LEN, M_IPFW, M_WAITOK | M_ZERO);
 	IPFW_RLOCK(chain);
-	/* Serialize all the data. */
+retry:
+	gencnt = chain->gencnt;
+	/* Estimate memory amount */
 	LIST_FOREACH(n, &chain->nat, _next) {
 		nat_cnt++;
-		if (off + SOF_NAT >= NAT_BUF_LEN)
-			goto nospace;
-		bcopy(n, &data[off], SOF_NAT);
-		off += SOF_NAT;
+		len += sizeof(struct cfg_nat);
 		LIST_FOREACH(r, &n->redir_chain, _next) {
-			if (off + SOF_REDIR >= NAT_BUF_LEN)
-				goto nospace;
-			bcopy(r, &data[off], SOF_REDIR);
-			off += SOF_REDIR;
+			len += sizeof(struct cfg_redir);
+			LIST_FOREACH(s, &r->spool_chain, _next)
+				len += sizeof(struct cfg_spool);
+		}
+	}
+	IPFW_RUNLOCK(chain);
+
+	data = malloc(len, M_TEMP, M_WAITOK | M_ZERO);
+	bcopy(&nat_cnt, data, sizeof(nat_cnt));
+
+	nat_cnt = 0;
+	len = sizeof(nat_cnt);
+
+	IPFW_RLOCK(chain);
+	if (gencnt != chain->gencnt) {
+		free(data, M_TEMP);
+		goto retry;
+	}
+	/* Serialize all the data. */
+	LIST_FOREACH(n, &chain->nat, _next) {
+		bcopy(n, &data[len], sizeof(struct cfg_nat));
+		len += sizeof(struct cfg_nat);
+		LIST_FOREACH(r, &n->redir_chain, _next) {
+			bcopy(r, &data[len], sizeof(struct cfg_redir));
+			len += sizeof(struct cfg_redir);
 			LIST_FOREACH(s, &r->spool_chain, _next) {
-				if (off + SOF_SPOOL >= NAT_BUF_LEN)
-					goto nospace;
-				bcopy(s, &data[off], SOF_SPOOL);
-				off += SOF_SPOOL;
+				bcopy(s, &data[len], sizeof(struct cfg_spool));
+				len += sizeof(struct cfg_spool);
 			}
 		}
 	}
-	err = 0; /* all good */
-nospace:
 	IPFW_RUNLOCK(chain);
-	if (err == 0) {
-		bcopy(&nat_cnt, data, sizeof(nat_cnt));
-		sooptcopyout(sopt, data, NAT_BUF_LEN);
-	} else {
-		printf("serialized data buffer not big enough:"
-		    "please increase NAT_BUF_LEN\n");
-	}
-	free(data, M_IPFW);
-	return (err);
+
+	error = sooptcopyout(sopt, data, len);
+	free(data, M_TEMP);
+
+	return (error);
 }
 
 static int
diff --git a/sys/netinet/ipfw/ip_fw_private.h b/sys/netinet/ipfw/ip_fw_private.h
index 13df9c9..16ef46d 100644
--- a/sys/netinet/ipfw/ip_fw_private.h
+++ b/sys/netinet/ipfw/ip_fw_private.h
@@ -225,6 +225,7 @@ struct ip_fw_chain {
 	struct rwlock	uh_lock;	/* lock for upper half */
 #endif
 	uint32_t	id;		/* ruleset id */
+	uint32_t	gencnt;		/* generation count */
 };
 
 struct sockopt;	/* used by tcp_var.h */