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

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

The following reply was made to PR kern/54950; it has been noted by GNATS.

From: Lloyd Parkes <lloyd%must-have-coffee.gen.nz@localhost>
To: gnats-bugs%netbsd.org@localhost, kern-bug-people%netbsd.org@localhost,
 gnats-admin%netbsd.org@localhost, netbsd-bugs%netbsd.org@localhost
Cc: 
Subject: Re: PR/54950 CVS commit: src/sys/net/npf
Date: Thu, 13 Feb 2020 19:50:29 +1300

 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));