Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 8 Jul 2011 14:30:06 +0000 (UTC)
From:      Gleb Smirnoff <glebius@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-8@freebsd.org
Subject:   svn commit: r223872 - in stable/8/sys: netgraph netinet netinet/ipfw netinet/libalias
Message-ID:  <201107081430.p68EU6nb023586@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: glebius
Date: Fri Jul  8 14:30:06 2011
New Revision: 223872
URL: http://svn.freebsd.org/changeset/base/223872

Log:
  Merge from head/ 220800,220837,220914:
  
  Log:
    LibAliasInit() should allocate memory with M_WAITOK flag. Modify it
    and its callers.
  
  Log:
    - 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

Modified:
  stable/8/sys/netgraph/ng_nat.c
  stable/8/sys/netinet/ip_fw.h
  stable/8/sys/netinet/ipfw/ip_fw_nat.c
  stable/8/sys/netinet/ipfw/ip_fw_private.h
  stable/8/sys/netinet/libalias/alias_db.c
Directory Properties:
  stable/8/sys/   (props changed)
  stable/8/sys/amd64/include/xen/   (props changed)
  stable/8/sys/cddl/contrib/opensolaris/   (props changed)
  stable/8/sys/contrib/dev/acpica/   (props changed)
  stable/8/sys/contrib/pf/   (props changed)

Modified: stable/8/sys/netgraph/ng_nat.c
==============================================================================
--- stable/8/sys/netgraph/ng_nat.c	Fri Jul  8 14:26:42 2011	(r223871)
+++ stable/8/sys/netgraph/ng_nat.c	Fri Jul  8 14:30:06 2011	(r223872)
@@ -280,10 +280,6 @@ ng_nat_constructor(node_p node)
 
 	/* Init aliasing engine. */
 	priv->lib = LibAliasInit(NULL);
-	if (priv->lib == NULL) {
-		free(priv, M_NETGRAPH);
-		return (ENOMEM);
-	}
 
 	/* Set same ports on. */
 	(void )LibAliasSetMode(priv->lib, PKT_ALIAS_SAME_PORTS,

Modified: stable/8/sys/netinet/ip_fw.h
==============================================================================
--- stable/8/sys/netinet/ip_fw.h	Fri Jul  8 14:26:42 2011	(r223871)
+++ stable/8/sys/netinet/ip_fw.h	Fri Jul  8 14:30:06 2011	(r223872)
@@ -380,8 +380,6 @@ struct cfg_redir {
 };
 #endif
 
-#define NAT_BUF_LEN     1024
-
 #ifdef IPFW_INTERNAL
 /* Nat configuration data struct. */
 struct cfg_nat {

Modified: stable/8/sys/netinet/ipfw/ip_fw_nat.c
==============================================================================
--- stable/8/sys/netinet/ipfw/ip_fw_nat.c	Fri Jul  8 14:26:42 2011	(r223871)
+++ stable/8/sys/netinet/ipfw/ip_fw_nat.c	Fri Jul  8 14:30:06 2011	(r223872)
@@ -138,7 +138,7 @@ del_redir_spool_cfg(struct cfg_nat *n, s
 	}
 }
 
-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 cf
 		/* And finally hook this redir entry. */
 		LIST_INSERT_HEAD(&ptr->redir_chain, r, _next);
 	}
-	return (1);
 }
 
 static int
@@ -354,59 +353,57 @@ lookup_nat(struct nat_list *l, int nat_i
 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;
+	size_t len;
+	int gencnt, 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. */
-		ptr = malloc(sizeof(struct cfg_nat),
-		    M_IPFW, M_NOWAIT | M_ZERO);
-		if (ptr == NULL) {
-			IPFW_WUNLOCK(chain);
-			free(buf, M_IPFW);
-			return (ENOSPC);
-		}
+		ptr = malloc(sizeof(struct cfg_nat), M_IPFW, M_WAITOK | M_ZERO);
 		ptr->lib = LibAliasInit(NULL);
-		if (ptr->lib == NULL) {
-			IPFW_WUNLOCK(chain);
-			free(ptr, M_IPFW);
-			free(buf, M_IPFW);
-			return (EINVAL);
-		}
 		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);
 	}
-	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.
@@ -415,11 +412,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);
+	/* 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
@@ -449,52 +454,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

Modified: stable/8/sys/netinet/ipfw/ip_fw_private.h
==============================================================================
--- stable/8/sys/netinet/ipfw/ip_fw_private.h	Fri Jul  8 14:26:42 2011	(r223871)
+++ stable/8/sys/netinet/ipfw/ip_fw_private.h	Fri Jul  8 14:30:06 2011	(r223872)
@@ -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 */

Modified: stable/8/sys/netinet/libalias/alias_db.c
==============================================================================
--- stable/8/sys/netinet/libalias/alias_db.c	Fri Jul  8 14:26:42 2011	(r223871)
+++ stable/8/sys/netinet/libalias/alias_db.c	Fri Jul  8 14:30:06 2011	(r223872)
@@ -2492,9 +2492,14 @@ LibAliasInit(struct libalias *la)
 #endif
 
 	if (la == NULL) {
+#ifdef _KERNEL
+#undef malloc	/* XXX: ugly */
+		la = malloc(sizeof *la, M_ALIAS, M_WAITOK | M_ZERO);
+#else
 		la = calloc(sizeof *la, 1);
 		if (la == NULL)
 			return (la);
+#endif
 
 #ifndef	_KERNEL		/* kernel cleans up on module unload */
 		if (LIST_EMPTY(&instancehead))



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