Module Name:    src
Committed By:   rmind
Date:           Sun Feb  5 00:37:13 UTC 2012

Modified Files:
        src/lib/libnpf: npf.c npf.h
        src/sys/net/npf: npf.h npf_ctl.c npf_nat.c npf_processor.c
        src/usr.sbin/npf/npfctl: npf_build.c npfctl.c npfctl.h

Log Message:
Multiple NPF fixes, add better error reporting from kernel side, add some
asserts, bump the version.


To generate a diff of this commit:
cvs rdiff -u -r1.6 -r1.7 src/lib/libnpf/npf.c
cvs rdiff -u -r1.5 -r1.6 src/lib/libnpf/npf.h
cvs rdiff -u -r1.12 -r1.13 src/sys/net/npf/npf.h
cvs rdiff -u -r1.11 -r1.12 src/sys/net/npf/npf_ctl.c
cvs rdiff -u -r1.9 -r1.10 src/sys/net/npf/npf_nat.c
cvs rdiff -u -r1.8 -r1.9 src/sys/net/npf/npf_processor.c
cvs rdiff -u -r1.2 -r1.3 src/usr.sbin/npf/npfctl/npf_build.c
cvs rdiff -u -r1.9 -r1.10 src/usr.sbin/npf/npfctl/npfctl.c
cvs rdiff -u -r1.10 -r1.11 src/usr.sbin/npf/npfctl/npfctl.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/lib/libnpf/npf.c
diff -u src/lib/libnpf/npf.c:1.6 src/lib/libnpf/npf.c:1.7
--- src/lib/libnpf/npf.c:1.6	Sun Jan 15 00:49:47 2012
+++ src/lib/libnpf/npf.c	Sun Feb  5 00:37:13 2012
@@ -1,4 +1,4 @@
-/*	$NetBSD: npf.c,v 1.6 2012/01/15 00:49:47 rmind Exp $	*/
+/*	$NetBSD: npf.c,v 1.7 2012/02/05 00:37:13 rmind Exp $	*/
 
 /*-
  * Copyright (c) 2010-2012 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: npf.c,v 1.6 2012/01/15 00:49:47 rmind Exp $");
+__KERNEL_RCSID(0, "$NetBSD: npf.c,v 1.7 2012/02/05 00:37:13 rmind Exp $");
 
 #include <sys/types.h>
 #include <netinet/in_systm.h>
@@ -39,6 +39,7 @@ __KERNEL_RCSID(0, "$NetBSD: npf.c,v 1.6 
 
 #include <stdlib.h>
 #include <string.h>
+#include <assert.h>
 #include <errno.h>
 #include <err.h>
 
@@ -54,6 +55,8 @@ struct nl_config {
 	/* Priority counters. */
 	pri_t			ncf_rule_pri;
 	pri_t			ncf_nat_pri;
+	/* Error report. */
+	prop_dictionary_t	ncf_err;
 	/* Custom file to externalise property-list. */
 	const char *		ncf_plist;
 	bool			ncf_flush;
@@ -80,7 +83,7 @@ npf_config_create(void)
 {
 	nl_config_t *ncf;
 
-	ncf = malloc(sizeof(*ncf));
+	ncf = calloc(1, sizeof(*ncf));
 	if (ncf == NULL) {
 		return NULL;
 	}
@@ -101,8 +104,8 @@ npf_config_create(void)
 int
 npf_config_submit(nl_config_t *ncf, int fd)
 {
-	prop_dictionary_t npf_dict;
 	const char *plist = ncf->ncf_plist;
+	prop_dictionary_t npf_dict;
 	int error = 0;
 
 	npf_dict = prop_dictionary_create();
@@ -119,9 +122,19 @@ npf_config_submit(nl_config_t *ncf, int 
 		if (!prop_dictionary_externalize_to_file(npf_dict, plist)) {
 			error = errno;
 		}
-	} else {
-		error = prop_dictionary_send_ioctl(npf_dict, fd, IOC_NPF_RELOAD);
+		prop_object_release(npf_dict);
+		return error;
+	}
+
+	error = prop_dictionary_sendrecv_ioctl(npf_dict, fd,
+	    IOC_NPF_RELOAD, &ncf->ncf_err);
+	if (error) {
+		prop_object_release(npf_dict);
+		assert(ncf->ncf_err == NULL);
+		return error;
 	}
+
+	prop_dictionary_get_int32(ncf->ncf_err, "errno", &error);
 	prop_object_release(npf_dict);
 	return error;
 }
@@ -143,6 +156,21 @@ npf_config_flush(int fd)
 }
 
 void
+_npf_config_error(nl_config_t *ncf, nl_error_t *ne)
+{
+	memset(ne, 0, sizeof(*ne));
+	prop_dictionary_get_int32(ncf->ncf_err, "id", &ne->ne_id);
+	prop_dictionary_get_cstring(ncf->ncf_err,
+	    "source-file", &ne->ne_source_file);
+	prop_dictionary_get_uint32(ncf->ncf_err,
+	    "source-line", &ne->ne_source_line);
+	prop_dictionary_get_int32(ncf->ncf_err,
+	    "ncode-error", &ne->ne_ncode_error);
+	prop_dictionary_get_int32(ncf->ncf_err,
+	    "ncode-errat", &ne->ne_ncode_errat);
+}
+
+void
 npf_config_destroy(nl_config_t *ncf)
 {
 
@@ -150,6 +178,9 @@ npf_config_destroy(nl_config_t *ncf)
 	prop_object_release(ncf->ncf_rproc_list);
 	prop_object_release(ncf->ncf_table_list);
 	prop_object_release(ncf->ncf_nat_list);
+	if (ncf->ncf_err) {
+		prop_object_release(ncf->ncf_err);
+	}
 	free(ncf);
 }
 
@@ -531,10 +562,14 @@ npf_table_destroy(nl_table_t *tl)
 int
 npf_update_rule(int fd, const char *rname __unused, nl_rule_t *rl)
 {
-	prop_dictionary_t rldict = rl->nrl_dict;
+	prop_dictionary_t rldict = rl->nrl_dict, errdict = NULL;
 	int error;
 
-	error = prop_dictionary_send_ioctl(rldict, fd, IOC_NPF_UPDATE_RULE);
+	error = prop_dictionary_sendrecv_ioctl(rldict, fd,
+	    IOC_NPF_UPDATE_RULE, &errdict);
+	if (errdict) {
+		prop_object_release(errdict);
+	}
 	return error;
 }
 

Index: src/lib/libnpf/npf.h
diff -u src/lib/libnpf/npf.h:1.5 src/lib/libnpf/npf.h:1.6
--- src/lib/libnpf/npf.h:1.5	Sun Jan 15 00:49:47 2012
+++ src/lib/libnpf/npf.h	Sun Feb  5 00:37:13 2012
@@ -1,4 +1,4 @@
-/*	$NetBSD: npf.h,v 1.5 2012/01/15 00:49:47 rmind Exp $	*/
+/*	$NetBSD: npf.h,v 1.6 2012/02/05 00:37:13 rmind Exp $	*/
 
 /*-
  * Copyright (c) 2011 The NetBSD Foundation, Inc.
@@ -53,6 +53,18 @@ typedef struct nl_table		nl_table_t;
 
 typedef struct nl_rule		nl_nat_t;
 
+#ifdef _NPF_PRIVATE
+
+typedef struct {
+	int		ne_id;
+	char *		ne_source_file;
+	u_int		ne_source_line;
+	int		ne_ncode_error;
+	int		ne_ncode_errat;
+} nl_error_t;
+
+#endif
+
 #define	NPF_CODE_NCODE		1
 #define	NPF_CODE_BPF		2
 
@@ -65,6 +77,7 @@ int		npf_config_submit(nl_config_t *, in
 void		npf_config_destroy(nl_config_t *);
 int		npf_config_flush(int);
 #ifdef _NPF_PRIVATE
+void		_npf_config_error(nl_config_t *, nl_error_t *);
 void		_npf_config_setsubmit(nl_config_t *, const char *);
 #endif
 

Index: src/sys/net/npf/npf.h
diff -u src/sys/net/npf/npf.h:1.12 src/sys/net/npf/npf.h:1.13
--- src/sys/net/npf/npf.h:1.12	Sun Jan 15 00:49:48 2012
+++ src/sys/net/npf/npf.h	Sun Feb  5 00:37:13 2012
@@ -1,4 +1,4 @@
-/*	$NetBSD: npf.h,v 1.12 2012/01/15 00:49:48 rmind Exp $	*/
+/*	$NetBSD: npf.h,v 1.13 2012/02/05 00:37:13 rmind Exp $	*/
 
 /*-
  * Copyright (c) 2009-2011 The NetBSD Foundation, Inc.
@@ -49,7 +49,7 @@
 #include "testing.h"
 #endif
 
-#define	NPF_VERSION		3
+#define	NPF_VERSION		4
 
 /*
  * Public declarations and definitions.
@@ -309,11 +309,11 @@ typedef enum {
 
 #define	IOC_NPF_VERSION		_IOR('N', 100, int)
 #define	IOC_NPF_SWITCH		_IOW('N', 101, int)
-#define	IOC_NPF_RELOAD		_IOW('N', 102, struct plistref)
+#define	IOC_NPF_RELOAD		_IOWR('N', 102, struct plistref)
 #define	IOC_NPF_TABLE		_IOW('N', 103, struct npf_ioctl_table)
 #define	IOC_NPF_STATS		_IOW('N', 104, void *)
 #define	IOC_NPF_SESSIONS_SAVE	_IOR('N', 105, struct plistref)
 #define	IOC_NPF_SESSIONS_LOAD	_IOW('N', 106, struct plistref)
-#define	IOC_NPF_UPDATE_RULE	_IOW('N', 107, struct plistref)
+#define	IOC_NPF_UPDATE_RULE	_IOWR('N', 107, struct plistref)
 
 #endif	/* _NPF_NET_H_ */

Index: src/sys/net/npf/npf_ctl.c
diff -u src/sys/net/npf/npf_ctl.c:1.11 src/sys/net/npf/npf_ctl.c:1.12
--- src/sys/net/npf/npf_ctl.c:1.11	Sun Jan 15 00:49:48 2012
+++ src/sys/net/npf/npf_ctl.c	Sun Feb  5 00:37:13 2012
@@ -1,4 +1,4 @@
-/*	$NetBSD: npf_ctl.c,v 1.11 2012/01/15 00:49:48 rmind Exp $	*/
+/*	$NetBSD: npf_ctl.c,v 1.12 2012/02/05 00:37:13 rmind Exp $	*/
 
 /*-
  * Copyright (c) 2009-2011 The NetBSD Foundation, Inc.
@@ -37,7 +37,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: npf_ctl.c,v 1.11 2012/01/15 00:49:48 rmind Exp $");
+__KERNEL_RCSID(0, "$NetBSD: npf_ctl.c,v 1.12 2012/02/05 00:37:13 rmind Exp $");
 
 #include <sys/param.h>
 #include <sys/conf.h>
@@ -48,6 +48,14 @@ __KERNEL_RCSID(0, "$NetBSD: npf_ctl.c,v 
 #include "npf_ncode.h"
 #include "npf_impl.h"
 
+#if defined(DEBUG) || defined(DIAGNOSTIC)
+#define	NPF_ERR_DEBUG(e) \
+	prop_dictionary_set_cstring_nocopy((e), "source-file", __FILE__); \
+	prop_dictionary_set_uint32((e), "source-line", __LINE__);
+#else
+#define	NPF_ERR_DEBUG(e)
+#endif
+
 /*
  * npfctl_switch: enable or disable packet inspection.
  */
@@ -69,15 +77,18 @@ npfctl_switch(void *data)
 }
 
 static int __noinline
-npf_mk_tables(npf_tableset_t *tblset, prop_array_t tables)
+npf_mk_tables(npf_tableset_t *tblset, prop_array_t tables,
+    prop_dictionary_t errdict)
 {
 	prop_object_iterator_t it;
 	prop_dictionary_t tbldict;
 	int error = 0;
 
 	/* Tables - array. */
-	if (prop_object_type(tables) != PROP_TYPE_ARRAY)
+	if (prop_object_type(tables) != PROP_TYPE_ARRAY) {
+		NPF_ERR_DEBUG(errdict);
 		return EINVAL;
+	}
 
 	it = prop_array_iterator(tables);
 	while ((tbldict = prop_object_iterator_next(it)) != NULL) {
@@ -90,6 +101,7 @@ npf_mk_tables(npf_tableset_t *tblset, pr
 
 		/* Table - dictionary. */
 		if (prop_object_type(tbldict) != PROP_TYPE_DICTIONARY) {
+			NPF_ERR_DEBUG(errdict);
 			error = EINVAL;
 			break;
 		}
@@ -98,7 +110,7 @@ npf_mk_tables(npf_tableset_t *tblset, pr
 		prop_dictionary_get_uint32(tbldict, "id", &tid);
 		prop_dictionary_get_int32(tbldict, "type", &type);
 
-		/* Validate them. */
+		/* Validate them, check for duplicate IDs. */
 		error = npf_table_check(tblset, tid, type);
 		if (error)
 			break;
@@ -106,6 +118,7 @@ npf_mk_tables(npf_tableset_t *tblset, pr
 		/* Create and insert the table. */
 		t = npf_table_create(tid, type, 1024);	/* XXX */
 		if (t == NULL) {
+			NPF_ERR_DEBUG(errdict);
 			error = ENOMEM;
 			break;
 		}
@@ -115,6 +128,7 @@ npf_mk_tables(npf_tableset_t *tblset, pr
 		/* Entries. */
 		entries = prop_dictionary_get(tbldict, "entries");
 		if (prop_object_type(entries) != PROP_TYPE_ARRAY) {
+			NPF_ERR_DEBUG(errdict);
 			error = EINVAL;
 			break;
 		}
@@ -153,6 +167,7 @@ npf_mk_rproc(prop_array_t rprocs, const 
 	while ((rpdict = prop_object_iterator_next(it)) != NULL) {
 		const char *iname;
 		prop_dictionary_get_cstring_nocopy(rpdict, "name", &iname);
+		KASSERT(iname != NULL);
 		if (strcmp(rpname, iname) == 0)
 			break;
 	}
@@ -170,42 +185,65 @@ npf_mk_rproc(prop_array_t rprocs, const 
 }
 
 static int __noinline
-npf_mk_singlerule(prop_dictionary_t rldict, prop_array_t rps, npf_rule_t **rl)
+npf_mk_ncode(prop_object_t obj, void **code, size_t *csize,
+    prop_dictionary_t errdict)
+{
+	const void *ncptr;
+	int nc_err, errat;
+	size_t nc_size;
+	void *nc;
+
+	/*
+	 * Allocate, copy and validate n-code. XXX: Inefficient.
+	 */
+	ncptr = prop_data_data_nocopy(obj);
+	nc_size = prop_data_size(obj);
+	if (ncptr == NULL || nc_size > NPF_NCODE_LIMIT) {
+		NPF_ERR_DEBUG(errdict);
+		return ERANGE;
+	}
+	nc = npf_ncode_alloc(nc_size);
+	if (nc == NULL) {
+		NPF_ERR_DEBUG(errdict);
+		return ENOMEM;
+	}
+	memcpy(nc, ncptr, nc_size);
+	nc_err = npf_ncode_validate(nc, nc_size, &errat);
+	if (nc_err) {
+		npf_ncode_free(nc, nc_size);
+		prop_dictionary_set_int32(errdict, "ncode-error", nc_err);
+		prop_dictionary_set_int32(errdict, "ncode-errat", errat);
+		return EINVAL;
+	}
+	*code = nc;
+	*csize = nc_size;
+	return 0;
+}
+
+static int __noinline
+npf_mk_singlerule(prop_dictionary_t rldict, prop_array_t rps, npf_rule_t **rl,
+    prop_dictionary_t errdict)
 {
 	const char *rnm;
 	npf_rproc_t *rp;
 	prop_object_t obj;
 	size_t nc_size;
 	void *nc;
+	int p, error;
 
 	/* Rule - dictionary. */
-	if (prop_object_type(rldict) != PROP_TYPE_DICTIONARY)
+	if (prop_object_type(rldict) != PROP_TYPE_DICTIONARY) {
+		NPF_ERR_DEBUG(errdict);
 		return EINVAL;
+	}
 
-	/* N-code (binary data). */
+	error = 0;
 	obj = prop_dictionary_get(rldict, "ncode");
 	if (obj) {
-		const void *ncptr;
-		int npf_err, errat;
-
-		/*
-		 * Allocate, copy and validate n-code. XXX: Inefficient.
-		 */
-		ncptr = prop_data_data_nocopy(obj);
-		nc_size = prop_data_size(obj);
-		if (ncptr == NULL || nc_size > NPF_NCODE_LIMIT) {
-			return EINVAL;
-		}
-		nc = npf_ncode_alloc(nc_size);
-		if (nc == NULL) {
-			return EINVAL;
-		}
-		memcpy(nc, ncptr, nc_size);
-		npf_err = npf_ncode_validate(nc, nc_size, &errat);
-		if (npf_err) {
-			npf_ncode_free(nc, nc_size);
-			/* TODO: return error details via proplib */
-			return EINVAL;
+		/* N-code (binary data). */
+		error = npf_mk_ncode(obj, &nc, &nc_size, errdict);
+		if (error) {
+			goto err;
 		}
 	} else {
 		/* No n-code. */
@@ -220,7 +258,9 @@ npf_mk_singlerule(prop_dictionary_t rldi
 			if (nc) {
 				npf_ncode_free(nc, nc_size);	/* XXX */
 			}
-			return EINVAL;
+			NPF_ERR_DEBUG(errdict);
+			error = EINVAL;
+			goto err;
 		}
 	} else {
 		rp = NULL;
@@ -228,23 +268,30 @@ npf_mk_singlerule(prop_dictionary_t rldi
 
 	/* Finally, allocate and return the rule. */
 	*rl = npf_rule_alloc(rldict, rp, nc, nc_size);
+	KASSERT(*rl != NULL);
 	return 0;
+err:
+	prop_dictionary_get_int32(rldict, "priority", &p); /* XXX */
+	prop_dictionary_set_int32(errdict, "id", p);
+	return error;
 }
 
 static int __noinline
-npf_mk_subrules(npf_ruleset_t *rlset, prop_array_t rules, prop_array_t rprocs)
+npf_mk_subrules(npf_ruleset_t *rlset, prop_array_t rules, prop_array_t rprocs,
+    prop_dictionary_t errdict)
 {
 	prop_object_iterator_t it;
 	prop_dictionary_t rldict;
 	int error = 0;
 
 	if (prop_object_type(rules) != PROP_TYPE_ARRAY) {
+		NPF_ERR_DEBUG(errdict);
 		return EINVAL;
 	}
 	it = prop_array_iterator(rules);
 	while ((rldict = prop_object_iterator_next(it)) != NULL) {
 		npf_rule_t *rl;
-		error = npf_mk_singlerule(rldict, rprocs, &rl);
+		error = npf_mk_singlerule(rldict, rprocs, &rl, errdict);
 		if (error) {
 			break;
 		}
@@ -255,7 +302,8 @@ npf_mk_subrules(npf_ruleset_t *rlset, pr
 }
 
 static int __noinline
-npf_mk_rules(npf_ruleset_t *rlset, prop_array_t rules, prop_array_t rprocs)
+npf_mk_rules(npf_ruleset_t *rlset, prop_array_t rules, prop_array_t rprocs,
+    prop_dictionary_t errdict)
 {
 	prop_object_iterator_t it;
 	prop_dictionary_t rldict, rpdict;
@@ -263,13 +311,16 @@ npf_mk_rules(npf_ruleset_t *rlset, prop_
 
 	/* Rule procedures and the ruleset - arrays. */
 	if (prop_object_type(rprocs) != PROP_TYPE_ARRAY ||
-	    prop_object_type(rules) != PROP_TYPE_ARRAY)
+	    prop_object_type(rules) != PROP_TYPE_ARRAY) {
+		NPF_ERR_DEBUG(errdict);
 		return EINVAL;
+	}
 
 	it = prop_array_iterator(rprocs);
 	while ((rpdict = prop_object_iterator_next(it)) != NULL) {
 		if (prop_dictionary_get(rpdict, "rproc-ptr")) {
 			prop_object_iterator_release(it);
+			NPF_ERR_DEBUG(errdict);
 			return EINVAL;
 		}
 	}
@@ -283,7 +334,7 @@ npf_mk_rules(npf_ruleset_t *rlset, prop_
 		npf_rule_t *rl;
 
 		/* Generate a single rule. */
-		error = npf_mk_singlerule(rldict, rprocs, &rl);
+		error = npf_mk_singlerule(rldict, rprocs, &rl, errdict);
 		if (error) {
 			break;
 		}
@@ -296,7 +347,7 @@ npf_mk_rules(npf_ruleset_t *rlset, prop_
 			continue;
 		}
 		rlsetsub = npf_rule_subset(rl);
-		error = npf_mk_subrules(rlsetsub, subrules, rprocs);
+		error = npf_mk_subrules(rlsetsub, subrules, rprocs, errdict);
 		if (error)
 			break;
 	}
@@ -308,15 +359,18 @@ npf_mk_rules(npf_ruleset_t *rlset, prop_
 }
 
 static int __noinline
-npf_mk_natlist(npf_ruleset_t *nset, prop_array_t natlist)
+npf_mk_natlist(npf_ruleset_t *nset, prop_array_t natlist,
+    prop_dictionary_t errdict)
 {
 	prop_object_iterator_t it;
 	prop_dictionary_t natdict;
 	int error;
 
 	/* NAT policies - array. */
-	if (prop_object_type(natlist) != PROP_TYPE_ARRAY)
+	if (prop_object_type(natlist) != PROP_TYPE_ARRAY) {
+		NPF_ERR_DEBUG(errdict);
 		return EINVAL;
+	}
 
 	error = 0;
 	it = prop_array_iterator(natlist);
@@ -326,6 +380,7 @@ npf_mk_natlist(npf_ruleset_t *nset, prop
 
 		/* NAT policy - dictionary. */
 		if (prop_object_type(natdict) != PROP_TYPE_DICTIONARY) {
+			NPF_ERR_DEBUG(errdict);
 			error = EINVAL;
 			break;
 		}
@@ -334,7 +389,7 @@ npf_mk_natlist(npf_ruleset_t *nset, prop
 		 * NAT policies are standard rules, plus additional
 		 * information for translation.  Make a rule.
 		 */
-		error = npf_mk_singlerule(natdict, NULL, &rl);
+		error = npf_mk_singlerule(natdict, NULL, &rl, errdict);
 		if (error) {
 			break;
 		}
@@ -349,7 +404,7 @@ npf_mk_natlist(npf_ruleset_t *nset, prop
 		/* Allocate a new NAT policy and assign to the rule. */
 		np = npf_nat_newpolicy(natdict, nset);
 		if (np == NULL) {
-			npf_rule_free(rl);
+			NPF_ERR_DEBUG(errdict);
 			error = ENOMEM;
 			break;
 		}
@@ -370,12 +425,12 @@ npf_mk_natlist(npf_ruleset_t *nset, prop
 int
 npfctl_reload(u_long cmd, void *data)
 {
-	const struct plistref *pref = data;
+	struct plistref *pref = data;
+	prop_dictionary_t dict, errdict;
 	prop_array_t natlist, tables, rprocs, rules;
 	npf_tableset_t *tblset = NULL;
 	npf_ruleset_t *rlset = NULL;
 	npf_ruleset_t *nset = NULL;
-	prop_dictionary_t dict;
 	bool flush;
 	int error;
 
@@ -389,10 +444,13 @@ npfctl_reload(u_long cmd, void *data)
 	if (dict == NULL)
 		return EINVAL;
 #endif
+	/* Dictionary for error reporting. */
+	errdict = prop_dictionary_create();
+
 	/* NAT policies. */
 	nset = npf_ruleset_create();
 	natlist = prop_dictionary_get(dict, "translation");
-	error = npf_mk_natlist(nset, natlist);
+	error = npf_mk_natlist(nset, natlist, errdict);
 	if (error) {
 		goto fail;
 	}
@@ -400,7 +458,7 @@ npfctl_reload(u_long cmd, void *data)
 	/* Tables. */
 	tblset = npf_tableset_create();
 	tables = prop_dictionary_get(dict, "tables");
-	error = npf_mk_tables(tblset, tables);
+	error = npf_mk_tables(tblset, tables, errdict);
 	if (error) {
 		goto fail;
 	}
@@ -409,7 +467,7 @@ npfctl_reload(u_long cmd, void *data)
 	rlset = npf_ruleset_create();
 	rprocs = prop_dictionary_get(dict, "rprocs");
 	rules = prop_dictionary_get(dict, "rules");
-	error = npf_mk_rules(rlset, rules, rprocs);
+	error = npf_mk_rules(rlset, rules, rprocs, errdict);
 	if (error) {
 		goto fail;
 	}
@@ -445,7 +503,12 @@ fail:
 		npf_tableset_destroy(tblset);
 	}
 	prop_object_release(dict);
-	return error;
+
+	/* Error report. */
+	prop_dictionary_set_int32(errdict, "errno", error);
+	prop_dictionary_copyout_ioctl(pref, cmd, errdict);
+	prop_object_release(errdict);
+	return 0;
 }
 
 /*
@@ -454,8 +517,8 @@ fail:
 int
 npfctl_update_rule(u_long cmd, void *data)
 {
-	const struct plistref *pref = data;
-	prop_dictionary_t dict;
+	struct plistref *pref = data;
+	prop_dictionary_t dict, errdict;
 	prop_array_t subrules;
 	prop_object_t obj;
 	npf_ruleset_t *rlset;
@@ -473,10 +536,14 @@ npfctl_update_rule(u_long cmd, void *dat
 	if (dict == NULL)
 		return EINVAL;
 #endif
+
+	/* Dictionary for error reporting. */
+	errdict = prop_dictionary_create();
+
 	/* Create the ruleset and construct sub-rules. */
 	rlset = npf_ruleset_create();
 	subrules = prop_dictionary_get(dict, "subrules");
-	error = npf_mk_subrules(rlset, subrules, NULL);
+	error = npf_mk_subrules(rlset, subrules, NULL, errdict);
 	if (error) {
 		goto out;
 	}
@@ -491,6 +558,11 @@ out:		/* Error path. */
 		npf_ruleset_destroy(rlset);
 	}
 	prop_object_release(dict);
+
+	/* Error report. */
+	prop_dictionary_set_int32(errdict, "errno", error);
+	prop_dictionary_copyout_ioctl(pref, cmd, errdict);
+	prop_object_release(errdict);
 	return error;
 }
 

Index: src/sys/net/npf/npf_nat.c
diff -u src/sys/net/npf/npf_nat.c:1.9 src/sys/net/npf/npf_nat.c:1.10
--- src/sys/net/npf/npf_nat.c:1.9	Sun Jan 15 00:49:48 2012
+++ src/sys/net/npf/npf_nat.c	Sun Feb  5 00:37:13 2012
@@ -1,4 +1,4 @@
-/*	$NetBSD: npf_nat.c,v 1.9 2012/01/15 00:49:48 rmind Exp $	*/
+/*	$NetBSD: npf_nat.c,v 1.10 2012/02/05 00:37:13 rmind Exp $	*/
 
 /*-
  * Copyright (c) 2010-2011 The NetBSD Foundation, Inc.
@@ -76,7 +76,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: npf_nat.c,v 1.9 2012/01/15 00:49:48 rmind Exp $");
+__KERNEL_RCSID(0, "$NetBSD: npf_nat.c,v 1.10 2012/02/05 00:37:13 rmind Exp $");
 
 #include <sys/param.h>
 #include <sys/kernel.h>
@@ -181,14 +181,19 @@ npf_nat_newpolicy(prop_dictionary_t natd
 	npf_portmap_t *pm;
 
 	np = kmem_zalloc(sizeof(npf_natpolicy_t), KM_SLEEP);
-	mutex_init(&np->n_lock, MUTEX_DEFAULT, IPL_SOFTNET);
-	cv_init(&np->n_cv, "npfnatcv");
-	LIST_INIT(&np->n_nat_list);
 
 	/* Translation type and flags. */
 	prop_dictionary_get_int32(natdict, "type", &np->n_type);
 	prop_dictionary_get_uint32(natdict, "flags", &np->n_flags);
-	KASSERT(np->n_type == NPF_NATIN || np->n_type == NPF_NATOUT);
+
+	/* Should be exclusively either inbound or outbound NAT. */
+	if (((np->n_type == NPF_NATIN) ^ (np->n_type == NPF_NATOUT)) == 0) {
+		kmem_free(np, sizeof(npf_natpolicy_t));
+		return NULL;
+	}
+	mutex_init(&np->n_lock, MUTEX_DEFAULT, IPL_SOFTNET);
+	cv_init(&np->n_cv, "npfnatcv");
+	LIST_INIT(&np->n_nat_list);
 
 	/* Translation IP. */
 	obj = prop_dictionary_get(natdict, "translation-ip");

Index: src/sys/net/npf/npf_processor.c
diff -u src/sys/net/npf/npf_processor.c:1.8 src/sys/net/npf/npf_processor.c:1.9
--- src/sys/net/npf/npf_processor.c:1.8	Sun Jan 15 00:49:49 2012
+++ src/sys/net/npf/npf_processor.c	Sun Feb  5 00:37:13 2012
@@ -1,4 +1,4 @@
-/*	$NetBSD: npf_processor.c,v 1.8 2012/01/15 00:49:49 rmind Exp $	*/
+/*	$NetBSD: npf_processor.c,v 1.9 2012/02/05 00:37:13 rmind Exp $	*/
 
 /*-
  * Copyright (c) 2009-2010 The NetBSD Foundation, Inc.
@@ -54,7 +54,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: npf_processor.c,v 1.8 2012/01/15 00:49:49 rmind Exp $");
+__KERNEL_RCSID(0, "$NetBSD: npf_processor.c,v 1.9 2012/02/05 00:37:13 rmind Exp $");
 
 #include <sys/param.h>
 #include <sys/kernel.h>
@@ -452,7 +452,7 @@ jmp_check:
 		error = nc_ptr_check(&iptr, nc, sz, 3, NULL, 0);
 		break;
 	case NPF_OPCODE_IP4MASK:
-		error = nc_ptr_check(&iptr, nc, sz, 3, &val, 1);
+		error = nc_ptr_check(&iptr, nc, sz, 3, &val, 3);
 		if (error) {
 			return error;
 		}
@@ -461,7 +461,7 @@ jmp_check:
 		}
 		break;
 	case NPF_OPCODE_IP6MASK:
-		error = nc_ptr_check(&iptr, nc, sz, 6, &val, 1);
+		error = nc_ptr_check(&iptr, nc, sz, 6, &val, 6);
 		if (error) {
 			return error;
 		}
@@ -507,12 +507,13 @@ static inline int
 nc_jmp_check(const void *nc, size_t sz, const uintptr_t jaddr)
 {
 	uintptr_t iaddr = (uintptr_t)nc;
-	size_t _jmp, adv;
-	bool _ret;
 	int error;
 
 	KASSERT(iaddr != jaddr);
 	do {
+		size_t _jmp, adv;
+		bool _ret;
+
 		error = nc_insn_check(iaddr, nc, sz, &adv, &_jmp, &_ret);
 		if (error) {
 			break;

Index: src/usr.sbin/npf/npfctl/npf_build.c
diff -u src/usr.sbin/npf/npfctl/npf_build.c:1.2 src/usr.sbin/npf/npfctl/npf_build.c:1.3
--- src/usr.sbin/npf/npfctl/npf_build.c:1.2	Sun Jan 15 00:49:48 2012
+++ src/usr.sbin/npf/npfctl/npf_build.c	Sun Feb  5 00:37:13 2012
@@ -1,4 +1,4 @@
-/*	$NetBSD: npf_build.c,v 1.2 2012/01/15 00:49:48 rmind Exp $	*/
+/*	$NetBSD: npf_build.c,v 1.3 2012/02/05 00:37:13 rmind Exp $	*/
 
 /*-
  * Copyright (c) 2011-2012 The NetBSD Foundation, Inc.
@@ -34,7 +34,7 @@
  */
 
 #include <sys/cdefs.h>
-__RCSID("$NetBSD: npf_build.c,v 1.2 2012/01/15 00:49:48 rmind Exp $");
+__RCSID("$NetBSD: npf_build.c,v 1.3 2012/02/05 00:37:13 rmind Exp $");
 
 #include <sys/types.h>
 #include <sys/ioctl.h>
@@ -75,6 +75,11 @@ npfctl_config_send(int fd)
 		errx(EXIT_FAILURE, "default group was not defined");
 	}
 	error = npf_config_submit(npf_conf, fd);
+	if (error) {
+		nl_error_t ne;
+		_npf_config_error(npf_conf, &ne);
+		npfctl_print_error(&ne);
+	}
 	npf_config_destroy(npf_conf);
 	return error;
 }

Index: src/usr.sbin/npf/npfctl/npfctl.c
diff -u src/usr.sbin/npf/npfctl/npfctl.c:1.9 src/usr.sbin/npf/npfctl/npfctl.c:1.10
--- src/usr.sbin/npf/npfctl/npfctl.c:1.9	Sun Jan 15 00:49:48 2012
+++ src/usr.sbin/npf/npfctl/npfctl.c	Sun Feb  5 00:37:13 2012
@@ -1,4 +1,4 @@
-/*	$NetBSD: npfctl.c,v 1.9 2012/01/15 00:49:48 rmind Exp $	*/
+/*	$NetBSD: npfctl.c,v 1.10 2012/02/05 00:37:13 rmind Exp $	*/
 
 /*-
  * Copyright (c) 2009-2012 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
  */
 
 #include <sys/cdefs.h>
-__RCSID("$NetBSD: npfctl.c,v 1.9 2012/01/15 00:49:48 rmind Exp $");
+__RCSID("$NetBSD: npfctl.c,v 1.10 2012/02/05 00:37:13 rmind Exp $");
 
 #include <sys/ioctl.h>
 #include <sys/stat.h>
@@ -207,6 +207,31 @@ npfctl_print_stats(int fd)
 	return 0;
 }
 
+void
+npfctl_print_error(const nl_error_t *ne)
+{
+	static const char *ncode_errors[] = {
+		[-NPF_ERR_OPCODE]	= "invalid instruction",
+		[-NPF_ERR_JUMP]		= "invalid jump",
+		[-NPF_ERR_REG]		= "invalid register",
+		[-NPF_ERR_INVAL]	= "invalid argument value",
+		[-NPF_ERR_RANGE]	= "processing out of range"
+	};
+	const int nc_err = ne->ne_ncode_error;
+	const char *srcfile = ne->ne_source_file;
+
+	if (srcfile) {
+		warnx("source %s line %d", srcfile, ne->ne_source_line);
+	}
+	if (nc_err) {
+		warnx("n-code error (%d): %s at offset 0x%x",
+		    nc_err, ncode_errors[-nc_err], ne->ne_ncode_errat);
+	}
+	if (ne->ne_id) {
+		warnx("object: %d", ne->ne_id);
+	}
+}
+
 static void
 npfctl(int action, int argc, char **argv)
 {

Index: src/usr.sbin/npf/npfctl/npfctl.h
diff -u src/usr.sbin/npf/npfctl/npfctl.h:1.10 src/usr.sbin/npf/npfctl/npfctl.h:1.11
--- src/usr.sbin/npf/npfctl/npfctl.h:1.10	Sun Jan 15 00:49:48 2012
+++ src/usr.sbin/npf/npfctl/npfctl.h	Sun Feb  5 00:37:13 2012
@@ -1,4 +1,4 @@
-/*	$NetBSD: npfctl.h,v 1.10 2012/01/15 00:49:48 rmind Exp $	*/
+/*	$NetBSD: npfctl.h,v 1.11 2012/02/05 00:37:13 rmind Exp $	*/
 
 /*-
  * Copyright (c) 2009-2012 The NetBSD Foundation, Inc.
@@ -91,6 +91,7 @@ void *		xrealloc(void *, size_t);
 char *		xstrdup(const char *);
 char *		xstrndup(const char *, size_t);
 
+void		npfctl_print_error(const nl_error_t *);
 bool		npfctl_table_exists_p(const char *);
 in_port_t	npfctl_portno(const char *);
 uint8_t		npfctl_icmpcode(uint8_t, const char *);

Reply via email to