NetBSD Problem Report #55403

From mlelstv@tazz.1st.de  Sat Jun 20 15:31:59 2020
Return-Path: <mlelstv@tazz.1st.de>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 54C861A9217
	for <gnats-bugs@gnats.NetBSD.org>; Sat, 20 Jun 2020 15:31:59 +0000 (UTC)
Message-Id: <20200620152621.B5F07CCAE7@tazz.1st.de>
Date: Sat, 20 Jun 2020 17:26:21 +0200 (CEST)
From: mlelstv@serpens.de
Reply-To: mlelstv@serpens.de
To: gnats-bugs@NetBSD.org
Subject: npfctl miscompiles IPv6 rules
X-Send-Pr-Version: 3.95

>Number:         55403
>Notify-List:    kim, riastradh
>Category:       bin
>Synopsis:       npfctl miscompiles IPv6 rules
>Confidential:   no
>Severity:       critical
>Priority:       medium
>Responsible:    rmind
>State:          analyzed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat Jun 20 15:35:00 +0000 2020
>Closed-Date:    
>Last-Modified:  Sun Nov 17 14:52:06 +0000 2024
>Originator:     Michael van Elst
>Release:        NetBSD 9.99.64
>Organization:

>Environment:


System: NetBSD tazz 9.99.64 NetBSD 9.99.64 (GENERIC) #2: Sun May 31 21:57:01 UTC 2020 mlelstv@slowpoke:/scratch2/obj.amd64/scratch/netbsd-current/src/sys/arch/amd64/compile/GENERIC amd64
Architecture: x86_64
Machine: amd64
>Description:

The byte code generated from the configuration:


$primary_if = lo0
$list = { fe80::1, fe80::2 }
group "primary" on $primary_if {
  pass in final family inet6 proto tcp from $list
}

is mis-compiled to

(000) ld       M[0]
(001) jeq      #0x6             jt 2    jf 22
(002) ld       M[2]
(003) jeq      #0x6             jt 4    jf 22
(004) ld       [8]
(005) jeq      #0xfe800000      jt 21   jf 6
(006) ld       [12]
(007) jeq      #0x0             jt 21   jf 8
(008) ld       [16]
(009) jeq      #0x0             jt 21   jf 10
(010) ld       [20]
(011) jeq      #0x1             jt 21   jf 12
(012) ld       [8]
(013) jeq      #0xfe800000      jt 21   jf 14
(014) ld       [12]
(015) jeq      #0x0             jt 21   jf 16
(016) ld       [16]
(017) jeq      #0x0             jt 21   jf 18
(018) ld       [20]
(019) jeq      #0x2             jt 21   jf 20
(020) ret      #0
(021) ret      #-1
(022) ret      #0

The match succeeds when any single 32bit word of the address matches.

The same with just a single address compiles correctly.

$primary_if = lo0
$list = { fe80::1 }
group "primary" on $primary_if {
  pass in final family inet6 proto tcp from $list
}

is compiled to

(000) ld       M[0]
(001) jeq      #0x6             jt 2    jf 13
(002) ld       M[2]
(003) jeq      #0x6             jt 4    jf 13
(004) ld       [8]
(005) jeq      #0xfe800000      jt 6    jf 13
(006) ld       [12]
(007) jeq      #0x0             jt 8    jf 13
(008) ld       [16]
(009) jeq      #0x0             jt 10   jf 13
(010) ld       [20]
(011) jeq      #0x1             jt 12   jf 13
(012) ret      #-1
(013) ret      #0


The compiler generates code for an AND condition, i.e. 

match family
AND match protocol
AND match word1 of address
AND match word2 of address
AND match word3 of address
AND match word4 of address
-> success

When compiling a list of addresses the code is inverted
to produce an OR condition. For IPv4 that's fine. I.e.

    match word1 of address1
AND match word1 of address2

is inverted to

    NOT match word1 of address1
AND NOT match word1 of address2
-> failure

But for IPv6, each word match is treated individually as

    match word1 of address1
AND match word2 of address1
AND match word3 of address1
AND match word4 of address1
AND match word1 of address2
AND match word2 of address2
AND match word3 of address2
AND match word4 of address2
-> success

is inverted to

    NOT match word1 of address1
AND NOT match word2 of address1
AND NOT match word3 of address1
AND NOT match word4 of address1
AND NOT match word1 of address2
AND NOT match word2 of address2
AND NOT match word3 of address2
AND NOT match word4 of address2
-> failure

And that's obviously wrong.


>How-To-Repeat:

Build a rule that matches against a list of IPv6 addresses.


>Fix:

Split the rule into matches against single IPv6 addresses or use a table lookup.


>Release-Note:

>Audit-Trail:

Responsible-Changed-From-To: bin-bug-people->rmind
Responsible-Changed-By: rmind@NetBSD.org
Responsible-Changed-When: Sat, 20 Jun 2020 17:37:59 +0000
Responsible-Changed-Why:
Take.


From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/55403 CVS commit: src/usr.sbin/npf/npftest/libnpftest
Date: Tue, 29 Oct 2024 22:13:52 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Tue Oct 29 22:13:52 UTC 2024

 Modified Files:
 	src/usr.sbin/npf/npftest/libnpftest: npf_rule_test.c

 Log Message:
 npftest: Add AF_* parameter to test cases.

 No functional change intended.

 Preparation to add test cases for:

 PR bin/55403: npfctl miscompiles IPv6 rules


 To generate a diff of this commit:
 cvs rdiff -u -r1.19 -r1.20 \
     src/usr.sbin/npf/npftest/libnpftest/npf_rule_test.c

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

From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/55403 CVS commit: src
Date: Tue, 29 Oct 2024 22:24:31 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Tue Oct 29 22:24:31 UTC 2024

 Modified Files:
 	src/tests/net/npf: t_npf.sh
 	src/usr.sbin/npf/npftest: npftest.conf
 	src/usr.sbin/npf/npftest/libnpftest: npf_rule_test.c

 Log Message:
 npftest: Add a test to match groups of IPv6 addresses.

 The npf_rule test group is now an xfail.  (npftest doesn't have a way
 to mark individual cases in a test group as xfail, so this will have
 to do for now.)

 PR bin/55403: npfctl miscompiles IPv6 rules


 To generate a diff of this commit:
 cvs rdiff -u -r1.4 -r1.5 src/tests/net/npf/t_npf.sh
 cvs rdiff -u -r1.9 -r1.10 src/usr.sbin/npf/npftest/npftest.conf
 cvs rdiff -u -r1.20 -r1.21 \
     src/usr.sbin/npf/npftest/libnpftest/npf_rule_test.c

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

From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/55403 CVS commit: src
Date: Wed, 30 Oct 2024 10:12:32 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Wed Oct 30 10:12:32 UTC 2024

 Modified Files:
 	src/tests/net/npf: t_npf.sh
 	src/usr.sbin/npf/npftest: npftest.conf
 	src/usr.sbin/npf/npftest/libnpftest: npf_rule_test.c

 Log Message:
 npftest: Fix newly added test.

 - Adapt new test to actually exercise new rules.
 - Mark the right test xfail.

 PR bin/55403: npfctl miscompiles IPv6 rules


 To generate a diff of this commit:
 cvs rdiff -u -r1.5 -r1.6 src/tests/net/npf/t_npf.sh
 cvs rdiff -u -r1.10 -r1.11 src/usr.sbin/npf/npftest/npftest.conf
 cvs rdiff -u -r1.21 -r1.22 \
     src/usr.sbin/npf/npftest/libnpftest/npf_rule_test.c

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

From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/55403 CVS commit: src/usr.sbin/npf/npftest
Date: Wed, 30 Oct 2024 11:03:32 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Wed Oct 30 11:03:32 UTC 2024

 Modified Files:
 	src/usr.sbin/npf/npftest: npftest.conf
 	src/usr.sbin/npf/npftest/libnpftest: npf_rule_test.c

 Log Message:
 npftest: Expand test cases to cover more compiler paths.

 Cover masked ranges with full- and partial-word sizes.

 PR bin/55403: npfctl miscompiles IPv6 rules


 To generate a diff of this commit:
 cvs rdiff -u -r1.11 -r1.12 src/usr.sbin/npf/npftest/npftest.conf
 cvs rdiff -u -r1.22 -r1.23 \
     src/usr.sbin/npf/npftest/libnpftest/npf_rule_test.c

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

From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/55403 CVS commit: src
Date: Wed, 30 Oct 2024 11:19:38 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Wed Oct 30 11:19:38 UTC 2024

 Modified Files:
 	src/tests/net/npf: t_npf.sh
 	src/usr.sbin/npf/npfctl: npf_bpf_comp.c

 Log Message:
 npfctl(8): Fix compiling multiword comparisons, i.e., IPv6 addrs.

 PR bin/55403: npfctl miscompiles IPv6 rules


 To generate a diff of this commit:
 cvs rdiff -u -r1.6 -r1.7 src/tests/net/npf/t_npf.sh
 cvs rdiff -u -r1.16 -r1.17 src/usr.sbin/npf/npfctl/npf_bpf_comp.c

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

State-Changed-From-To: open->needs-pullups
State-Changed-By: riastradh@NetBSD.org
State-Changed-When: Wed, 30 Oct 2024 14:39:50 +0000
State-Changed-Why:
fix committed to HEAD, needs pullup-9 and pullup-10


State-Changed-From-To: needs-pullups->pending-pullups
State-Changed-By: riastradh@NetBSD.org
State-Changed-When: Sun, 10 Nov 2024 16:44:17 +0000
State-Changed-Why:
pullup-10 #1002 https://releng.netbsd.org/cgi-bin/req-10.cgi?show=1002
pullup-9 #1918 https://releng.netbsd.org/cgi-bin/req-9.cgi?show=1918


From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/55403 CVS commit: [netbsd-10] src/usr.sbin/npf
Date: Sun, 17 Nov 2024 13:18:59 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Sun Nov 17 13:18:59 UTC 2024

 Modified Files:
 	src/usr.sbin/npf/npfctl [netbsd-10]: npf_bpf_comp.c
 	src/usr.sbin/npf/npftest [netbsd-10]: npftest.conf
 	src/usr.sbin/npf/npftest/libnpftest [netbsd-10]: npf_rule_test.c

 Log Message:
 Pull up following revision(s) (requested by riastradh in ticket #1002):

 	usr.sbin/npf/npftest/npftest.conf: revision 1.10
 	usr.sbin/npf/npftest/npftest.conf: revision 1.11
 	usr.sbin/npf/npftest/npftest.conf: revision 1.12
 	usr.sbin/npf/npfctl/npf_bpf_comp.c: revision 1.17
 	usr.sbin/npf/npftest/libnpftest/npf_rule_test.c: revision 1.20
 	usr.sbin/npf/npftest/libnpftest/npf_rule_test.c: revision 1.21
 	usr.sbin/npf/npftest/libnpftest/npf_rule_test.c: revision 1.22
 	usr.sbin/npf/npftest/libnpftest/npf_rule_test.c: revision 1.23
 	tests/net/npf/t_npf.sh: revision 1.5
 	tests/net/npf/t_npf.sh: revision 1.6
 	tests/net/npf/t_npf.sh: revision 1.7

 npftest: Add AF_* parameter to test cases.
 No functional change intended.
 Preparation to add test cases for:
 PR bin/55403: npfctl miscompiles IPv6 rules

 npftest: Add a test to match groups of IPv6 addresses.
 The npf_rule test group is now an xfail.  (npftest doesn't have a way
 to mark individual cases in a test group as xfail, so this will have
 to do for now.)
 PR bin/55403: npfctl miscompiles IPv6 rules

 npftest: Fix newly added test.
 - Adapt new test to actually exercise new rules.
 - Mark the right test xfail.
 PR bin/55403: npfctl miscompiles IPv6 rules

 npftest: Expand test cases to cover more compiler paths.
 Cover masked ranges with full- and partial-word sizes.
 PR bin/55403: npfctl miscompiles IPv6 rules

 npfctl(8): Fix compiling multiword comparisons, i.e., IPv6 addrs.
 PR bin/55403: npfctl miscompiles IPv6 rules


 To generate a diff of this commit:
 cvs rdiff -u -r1.16 -r1.16.6.1 src/usr.sbin/npf/npfctl/npf_bpf_comp.c
 cvs rdiff -u -r1.9 -r1.9.6.1 src/usr.sbin/npf/npftest/npftest.conf
 cvs rdiff -u -r1.19 -r1.19.8.1 \
     src/usr.sbin/npf/npftest/libnpftest/npf_rule_test.c

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

From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/55403 CVS commit: [netbsd-9] src/usr.sbin/npf
Date: Sun, 17 Nov 2024 13:58:11 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Sun Nov 17 13:58:11 UTC 2024

 Modified Files:
 	src/usr.sbin/npf/npfctl [netbsd-9]: npf_bpf_comp.c
 	src/usr.sbin/npf/npftest [netbsd-9]: npftest.conf
 	src/usr.sbin/npf/npftest/libnpftest [netbsd-9]: npf_rule_test.c

 Log Message:
 Pull up following revision(s) (requested by riastradh in ticket #1918):

 	usr.sbin/npf/npftest/npftest.conf: revision 1.10
 	usr.sbin/npf/npftest/npftest.conf: revision 1.11
 	usr.sbin/npf/npftest/npftest.conf: revision 1.12
 	usr.sbin/npf/npfctl/npf_bpf_comp.c: revision 1.17
 	usr.sbin/npf/npftest/libnpftest/npf_rule_test.c: revision 1.20
 	usr.sbin/npf/npftest/libnpftest/npf_rule_test.c: revision 1.21
 	usr.sbin/npf/npftest/libnpftest/npf_rule_test.c: revision 1.22
 	usr.sbin/npf/npftest/libnpftest/npf_rule_test.c: revision 1.23
 	tests/net/npf/t_npf.sh: revision 1.5
 	tests/net/npf/t_npf.sh: revision 1.6
 	tests/net/npf/t_npf.sh: revision 1.7

 npftest: Add AF_* parameter to test cases.
 No functional change intended.
 Preparation to add test cases for:
 PR bin/55403: npfctl miscompiles IPv6 rules

 npftest: Add a test to match groups of IPv6 addresses.
 The npf_rule test group is now an xfail.  (npftest doesn't have a way
 to mark individual cases in a test group as xfail, so this will have
 to do for now.)
 PR bin/55403: npfctl miscompiles IPv6 rules

 npftest: Fix newly added test.
 - Adapt new test to actually exercise new rules.
 - Mark the right test xfail.
 PR bin/55403: npfctl miscompiles IPv6 rules

 npftest: Expand test cases to cover more compiler paths.
 Cover masked ranges with full- and partial-word sizes.
 PR bin/55403: npfctl miscompiles IPv6 rules

 npfctl(8): Fix compiling multiword comparisons, i.e., IPv6 addrs.
 PR bin/55403: npfctl miscompiles IPv6 rules


 To generate a diff of this commit:
 cvs rdiff -u -r1.13.2.3 -r1.13.2.4 src/usr.sbin/npf/npfctl/npf_bpf_comp.c
 cvs rdiff -u -r1.7.2.2 -r1.7.2.3 src/usr.sbin/npf/npftest/npftest.conf
 cvs rdiff -u -r1.17.2.2 -r1.17.2.3 \
     src/usr.sbin/npf/npftest/libnpftest/npf_rule_test.c

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

State-Changed-From-To: pending-pullups->analyzed
State-Changed-By: riastradh@NetBSD.org
State-Changed-When: Sun, 17 Nov 2024 14:52:06 +0000
State-Changed-Why:
fixed in HEAD, pulled up to 9 and 10
should be filed as a github issue with rmind too


>Unformatted:

Home	PR Database Search