From: Denis Ovsienko <denis@ovsienko.info>
Date: Sat, 25 Jan 2025 21:46:22 +0000 (+0000)
Subject: Use a 16-bit comparison for "pppoes NUM".
X-Git-Url: https://git.tcpdump.org/libpcap/commitdiff_plain/627fd121b60d984b4dc681a7a9fd5ee2e1f7161b

Use a 16-bit comparison for "pppoes NUM".

gen_pppoes() compares only the low 16 bits of a [32-bit] word, which
does not entirely match the problem space.  Eliminate the bitwise AND
instruction by comparing a [16-bit] half-word with the same 16 bits,
this produces simpler bytecode (thus the corresponding accept test
changes) with an equivalent behaviour (thus no apply tests change).
While at it, add a comment with the packet header diagram and use a
standard named constant for the maximum session ID value.
---

diff --git a/gencode.c b/gencode.c
index 1dbff96d..5c94e60d 100644
--- a/gencode.c
+++ b/gencode.c
@@ -9550,6 +9550,19 @@ gen_pppoed(compiler_state_t *cstate)
 	return gen_linktype(cstate, ETHERTYPE_PPPOED);
 }
 
+/*
+ * RFC 2516 Section 4:
+ *
+ * The Ethernet payload for PPPoE is as follows:
+ *
+ *                      1                   2                   3
+ *  0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |  VER  | TYPE  |      CODE     |          SESSION_ID           |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |            LENGTH             |           payload             ~
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ */
 struct block *
 gen_pppoes(compiler_state_t *cstate, bpf_u_int32 sess_num, int has_sess_num)
 {
@@ -9569,11 +9582,11 @@ gen_pppoes(compiler_state_t *cstate, bpf_u_int32 sess_num, int has_sess_num)
 
 	/* If a specific session is requested, check PPPoE session id */
 	if (has_sess_num) {
-		if (sess_num > 0x0000ffff) {
+		if (sess_num > UINT16_MAX) {
 			bpf_error(cstate, "PPPoE session number %u greater than maximum %u",
-			    sess_num, 0x0000ffff);
+			    sess_num, UINT16_MAX);
 		}
-		b1 = gen_mcmp(cstate, OR_LINKPL, 0, BPF_W, sess_num, 0x0000ffff);
+		b1 = gen_cmp(cstate, OR_LINKPL, 2, BPF_H, sess_num);
 		gen_and(b0, b1);
 		b0 = b1;
 	}
diff --git a/testprogs/TESTrun b/testprogs/TESTrun
index cdcdbb23..a79a18fe 100755
--- a/testprogs/TESTrun
+++ b/testprogs/TESTrun
@@ -3316,12 +3316,11 @@ my %accept_blocks = (
 		expr => 'pppoes 1234',
 		unopt => <<~'EOF',
 			(000) ldh      [12]
-			(001) jeq      #0x8864          jt 2	jf 6
-			(002) ld       [14]
-			(003) and      #0xffff
-			(004) jeq      #0x4d2           jt 5	jf 6
-			(005) ret      #200
-			(006) ret      #0
+			(001) jeq      #0x8864          jt 2	jf 5
+			(002) ldh      [16]
+			(003) jeq      #0x4d2           jt 4	jf 5
+			(004) ret      #200
+			(005) ret      #0
 			EOF
 	}, # pppoes_unary