Re: PR/54950 CVS commit: src/sys/net/npf

[Lloyd Parkes <lloyd%must-have-coffee.gen.nz@localhost>]

On 12/02/20 2:35 pm, Christos Zoulas wrote:
> The following reply was made to PR kern/54950; it has been noted by GNATS.
>
> From: "Christos Zoulas" <christos%netbsd.org@localhost>
> To: gnats-bugs%gnats.NetBSD.org@localhost
> Cc:
> Subject: PR/54950 CVS commit: src/sys/net/npf
> Date: Tue, 11 Feb 2020 20:34:55 -0500
>
>   Modified Files:
>   	src/sys/net/npf: npf_ruleset.c

I'm pretty sure that this doesn't cover all NULL pointer derefs in NPF. 
You can also get a NULL pointer deref for the NAT rulesets.

I'm still waiting for my CVS to rsync from anoncvs.netbsd.org so I don't 
actually know what this patch is, but a patch I generated last night is 
below.

On top of that, I don't see how any of this could have been triggered my 
my actual NPF configuration and I'm guessing that our trivial test cases 
have uncovered other problems. I'm going to try and regenerate my 
original configuration and see what happens.

Cheers

cvs diff: Diffing .
Index: npf_conf.c
===================================================================
RCS file: /vol/src/rsync-src/src/sys/net/npf/npf_conf.c,v
retrieving revision 1.15
diff -u -r1.15 npf_conf.c
--- npf_conf.c	25 Aug 2019 13:21:03 -0000	1.15
+++ npf_conf.c	12 Feb 2020 04:38:30 -0000
@@ -47,7 +47,7 @@

 #ifdef _KERNEL
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: npf_conf.c,v 1.14 2019/08/11 20:26:33 rmind 
Exp $");
+__KERNEL_RCSID(0, "$NetBSD: npf_conf.c,v 1.15 2019/08/25 13:21:03 rmind 
Exp $");

 #include <sys/param.h>
 #include <sys/types.h>
@@ -94,8 +94,8 @@
 	 * Note: the rulesets must be destroyed first, in order to drop
 	 * any references to the tableset.
 	 */
-	npf_ruleset_destroy(nc->ruleset);
-	npf_ruleset_destroy(nc->nat_ruleset);
+	if (nc->ruleset) npf_ruleset_destroy(nc->ruleset);
+	if (nc->nat_ruleset) npf_ruleset_destroy(nc->nat_ruleset);
 	npf_rprocset_destroy(nc->rule_procs);
 	npf_tableset_destroy(nc->tableset);
 	kmem_free(nc, sizeof(npf_config_t));