[PATCH 0/3] nftables: fix some endian issues

[PATCH 0/3] nftables: fix some endian issues

[Patrick McHardy]

These patches fix the endian issues Alvaro observed in the bridge table.

The main problem was that the ETH_P_* values are in host byte order, so
they never matches. This is fixed by simply converting the constants to
big endian.

Two related problems are that symbol table printing and higher layer
protocol lookup didn't take endianess into account properly. This is
fixes by exporting the value in the proper byte order before comparison.

With these patches, everything works as expected:

table bridge filter {
	chain input {
		 type filter hook input priority -200;
		 ip daddr 192.168.1.80 tcp dport ssh counter packets 0 bytes 0
		 ether type ip counter packets 128 bytes 36734
		 ether type arp counter packets 12 bytes 336
	}
}

[PATCH 1/3] payload: take endianess into account when updating the payload context

[Patrick McHardy]

payload_expr_pctx_update() uses the numeric protocol value in host byte
order to find the upper layer protocol. This obviously doesn't work for
protocol expressions in other byte orders, such as the ethernet protocol
on little endian.

Export the protocol value in the correct byte order and use that value
to look up the upper layer protocol.

Signed-off-by: Patrick McHardy <kaber@trash.net>
---
 src/payload.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/payload.c b/src/payload.c
index a1785a5..47861ed 100644
--- a/src/payload.c
+++ b/src/payload.c
@@ -69,13 +69,20 @@ static void payload_expr_pctx_update(struct proto_ctx *ctx,
 {
 	const struct expr *left = expr->left, *right = expr->right;
 	const struct proto_desc *base, *desc;
+	unsigned int proto = 0;
 
 	if (!(left->flags & EXPR_F_PROTOCOL))
 		return;
 
 	assert(expr->op == OP_EQ);
+
+	/* Export the data in the correct byte order */
+	assert(right->len / BITS_PER_BYTE <= sizeof(proto));
+	mpz_export_data(&proto, right->value, right->byteorder,
+			right->len / BITS_PER_BYTE);
+
 	base = ctx->protocol[left->payload.base].desc;
-	desc = proto_find_upper(base, mpz_get_uint32(right->value));
+	desc = proto_find_upper(base, proto);
 
 	proto_ctx_update(ctx, left->payload.base + 1, &expr->location, desc);
 }
-- 
1.9.3

[PATCH 2/3] datatype: take endianess into account in symbolic_constant_print()

[Patrick McHardy]

symbolic_constant_print() uses mpz_cmp_ui() to find the matching symbol.
Since GMP internally treats all values as being in host byte, this
doesn't work when the constant value is non-host byteorder, such as
the ethernet protocol type.

Export the expression's value in its original byteorder for comparison
to fix this.

Signed-off-by: Patrick McHardy <kaber@trash.net>
---
 src/datatype.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/datatype.c b/src/datatype.c
index 55af227..36d5985 100644
--- a/src/datatype.c
+++ b/src/datatype.c
@@ -132,9 +132,15 @@ void symbolic_constant_print(const struct symbol_table *tbl,
 			     const struct expr *expr)
 {
 	const struct symbolic_constant *s;
+	uint64_t val = 0;
+
+	/* Export the data in the correct byteorder for comparison */
+	assert(expr->len / BITS_PER_BYTE <= sizeof(val));
+	mpz_export_data(&val, expr->value, expr->byteorder,
+			expr->len / BITS_PER_BYTE);
 
 	for (s = tbl->symbols; s->identifier != NULL; s++) {
-		if (!mpz_cmp_ui(expr->value, s->value))
+		if (val == s->value)
 			break;
 	}
 
-- 
1.9.3

[PATCH 3/3] proto: fix byteorder of ETH_P_* values

[Patrick McHardy]

The ethernet header type is in big endian byte order, the ETH_P_* values
are in host byte order however. Fix this using __constant_htons().

Signed-off-by: Patrick McHardy <kaber@trash.net>
---
 src/proto.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/src/proto.c b/src/proto.c
index e5f49cb..15a456a 100644
--- a/src/proto.c
+++ b/src/proto.c
@@ -712,10 +712,10 @@ const struct proto_desc proto_vlan = {
 	.base		= PROTO_BASE_LL_HDR,
 	.protocol_key	= VLANHDR_TYPE,
 	.protocols	= {
-		PROTO_LINK(ETH_P_IP,		&proto_ip),
-		PROTO_LINK(ETH_P_ARP,		&proto_arp),
-		PROTO_LINK(ETH_P_IPV6,		&proto_ip6),
-		PROTO_LINK(ETH_P_8021Q,		&proto_vlan),
+		PROTO_LINK(__constant_htons(ETH_P_IP),		&proto_ip),
+		PROTO_LINK(__constant_htons(ETH_P_ARP),		&proto_arp),
+		PROTO_LINK(__constant_htons(ETH_P_IPV6),	&proto_ip6),
+		PROTO_LINK(__constant_htons(ETH_P_8021Q),	&proto_vlan),
 
 	},
 	.templates	= {
@@ -741,10 +741,10 @@ const struct datatype etheraddr_type = {
 
 static const struct symbol_table ethertype_tbl = {
 	.symbols	= {
-		SYMBOL("ip",		ETH_P_IP),
-		SYMBOL("arp",		ETH_P_ARP),
-		SYMBOL("ip6",		ETH_P_IPV6),
-		SYMBOL("vlan",		ETH_P_8021Q),
+		SYMBOL("ip",		__constant_htons(ETH_P_IP)),
+		SYMBOL("arp",		__constant_htons(ETH_P_ARP)),
+		SYMBOL("ip6",		__constant_htons(ETH_P_IPV6)),
+		SYMBOL("vlan",		__constant_htons(ETH_P_8021Q)),
 		SYMBOL_LIST_END
 	},
 };
@@ -792,10 +792,10 @@ const struct proto_desc proto_eth = {
 	.base		= PROTO_BASE_LL_HDR,
 	.protocol_key	= ETHHDR_TYPE,
 	.protocols	= {
-		PROTO_LINK(ETH_P_IP,		&proto_ip),
-		PROTO_LINK(ETH_P_ARP,		&proto_arp),
-		PROTO_LINK(ETH_P_IPV6,		&proto_ip6),
-		PROTO_LINK(ETH_P_8021Q,		&proto_vlan),
+		PROTO_LINK(__constant_htons(ETH_P_IP),		&proto_ip),
+		PROTO_LINK(__constant_htons(ETH_P_ARP),		&proto_arp),
+		PROTO_LINK(__constant_htons(ETH_P_IPV6),	&proto_ip6),
+		PROTO_LINK(__constant_htons(ETH_P_8021Q),	&proto_vlan),
 	},
 	.templates	= {
 		[ETHHDR_DADDR]		= ETHHDR_ADDR("daddr", ether_dhost),
-- 
1.9.3

Re: [PATCH 0/3] nftables: fix some endian issues

[Álvaro Neira Ayuso]

Hello Patrick

El 19/08/14 01:27, Patrick McHardy escribió:
> These patches fix the endian issues Alvaro observed in the bridge table.
>
> The main problem was that the ETH_P_* values are in host byte order, so
> they never matches. This is fixed by simply converting the constants to
> big endian.
>
> Two related problems are that symbol table printing and higher layer
> protocol lookup didn't take endianess into account properly. This is
> fixes by exporting the value in the proper byte order before comparison.
>
> With these patches, everything works as expected:
>
> table bridge filter {
> 	chain input {
> 		 type filter hook input priority -200;
> 		 ip daddr 192.168.1.80 tcp dport ssh counter packets 0 bytes 0
> 		 ether type ip counter packets 128 bytes 36734
> 		 ether type arp counter packets 12 bytes 336
> 	}
> }
>

I have been using the patches and it works perfectly. Thank you very much.

Alvaro
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html