diff mbox series

[iptables] ebtables: Fix among match

Message ID 20220928135524.22822-1-phil@nwl.cc
State Accepted
Delegated to: Florian Westphal
Headers show
Series [iptables] ebtables: Fix among match | expand

Commit Message

Phil Sutter Sept. 28, 2022, 1:55 p.m. UTC 
Fixed commit broke among match in two ways:

1) The two lookup sizes are 12 and 6, not 12 and 4 - among supports
   either ether+IP or ether only, not IP only.

2) Adding two to sreg_count to get the second register is too simple: It
   works only for four byte regs, not the 16 byte ones. The first
   register is always a 16 byte one, though.

Fixing (1) is trivial, fix (2) by introduction of nft_get_next_reg()
doing the right thing. For consistency, use it for among match creation,
too.

Fixes: f315af1cf8871 ("nft: track each register individually")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft-bridge.c |  4 ++--
 iptables/nft-shared.c | 16 ++++++++++++++++
 iptables/nft-shared.h |  5 +++++
 iptables/nft.c        |  6 ++----
 4 files changed, 25 insertions(+), 6 deletions(-)

Comments

Florian Westphal Sept. 28, 2022, 3:24 p.m. UTC | #1 
Phil Sutter <phil@nwl.cc> wrote:
> Fixed commit broke among match in two ways:
> 
> 1) The two lookup sizes are 12 and 6, not 12 and 4 - among supports
>    either ether+IP or ether only, not IP only.
> 
> 2) Adding two to sreg_count to get the second register is too simple: It
>    works only for four byte regs, not the 16 byte ones. The first
>    register is always a 16 byte one, though.
> 
> Fixing (1) is trivial, fix (2) by introduction of nft_get_next_reg()
> doing the right thing. For consistency, use it for among match creation,
> too.

LGTM, thanks.

Could you add a followup patch that cleans this up:

> diff --git a/iptables/nft-bridge.c b/iptables/nft-bridge.c
> index 659c5b58ba633..596dfdf8991f1 100644
> --- a/iptables/nft-bridge.c
> +++ b/iptables/nft-bridge.c
> @@ -349,7 +349,7 @@ static int lookup_analyze_payloads(struct nft_xt_ctx *ctx,
>  			return -1;
>  		}
>  
> -		sreg_count += 2;
> +		sreg_count = nft_get_next_reg(sreg_count, ETH_ALEN);


.. and renames sreg_count to 'enum nft_registers sreg' or similar?
diff mbox series

Patch

diff --git a/iptables/nft-bridge.c b/iptables/nft-bridge.c
index 659c5b58ba633..596dfdf8991f1 100644
--- a/iptables/nft-bridge.c
+++ b/iptables/nft-bridge.c
@@ -349,7 +349,7 @@  static int lookup_analyze_payloads(struct nft_xt_ctx *ctx,
 			return -1;
 		}
 
-		sreg_count += 2;
+		sreg_count = nft_get_next_reg(sreg_count, ETH_ALEN);
 
 		reg = nft_xt_ctx_get_sreg(ctx, sreg_count);
 		if (!reg) {
@@ -375,7 +375,7 @@  static int lookup_analyze_payloads(struct nft_xt_ctx *ctx,
 			return -1;
 		}
 		break;
-	case 4: /* ipv4addr */
+	case 6: /* ether */
 		val = lookup_check_ether_payload(reg->payload.base,
 						 reg->payload.offset,
 						 reg->payload.len);
diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c
index f8de2b715dd61..909fe6483205c 100644
--- a/iptables/nft-shared.c
+++ b/iptables/nft-shared.c
@@ -10,6 +10,7 @@ 
  * This code has been sponsored by Sophos Astaro <http://www.sophos.com>
  */
 
+#include <assert.h>
 #include <string.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -1603,3 +1604,18 @@  int nft_parse_hl(struct nft_xt_ctx *ctx,
 
 	return 0;
 }
+
+enum nft_registers nft_get_next_reg(enum nft_registers reg, size_t size)
+{
+	/* convert size to NETLINK_ALIGN-sized chunks */
+	size = (size + NETLINK_ALIGN - 1) / NETLINK_ALIGN;
+
+	/* map 16byte reg to 4byte one */
+	if (reg < __NFT_REG_MAX)
+		reg = NFT_REG32_00 + (reg - 1) * NFT_REG_SIZE / NFT_REG32_SIZE;
+
+	reg += size;
+	assert(reg <= NFT_REG32_15);
+
+	return reg;
+}
diff --git a/iptables/nft-shared.h b/iptables/nft-shared.h
index 8fcedcdd78fbe..c07d3270a407e 100644
--- a/iptables/nft-shared.h
+++ b/iptables/nft-shared.h
@@ -276,4 +276,9 @@  int nft_parse_hl(struct nft_xt_ctx *ctx, struct nftnl_expr *e, struct iptables_c
 #define min(x, y) ((x) < (y) ? (x) : (y))
 #define max(x, y) ((x) > (y) ? (x) : (y))
 
+/* simplified nftables:include/netlink.h, netlink_padded_len() */
+#define NETLINK_ALIGN		4
+
+enum nft_registers nft_get_next_reg(enum nft_registers reg, size_t size);
+
 #endif
diff --git a/iptables/nft.c b/iptables/nft.c
index 2165733ff74e9..09cb19c987322 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -1133,9 +1133,6 @@  gen_lookup(uint32_t sreg, const char *set_name, uint32_t set_id, uint32_t flags)
 	return e;
 }
 
-/* simplified nftables:include/netlink.h, netlink_padded_len() */
-#define NETLINK_ALIGN		4
-
 /* from nftables:include/datatype.h, TYPE_BITS */
 #define CONCAT_TYPE_BITS	6
 
@@ -1208,8 +1205,9 @@  static int __add_nft_among(struct nft_handle *h, const char *table,
 	nftnl_rule_add_expr(r, e);
 
 	if (ip) {
+		reg = nft_get_next_reg(reg, ETH_ALEN);
 		e = __gen_payload(NFT_PAYLOAD_NETWORK_HEADER, ip_addr_off[dst],
-				sizeof(struct in_addr), NFT_REG32_02);
+				  sizeof(struct in_addr), reg);
 		if (!e)
 			return -ENOMEM;
 		nftnl_rule_add_expr(r, e);