| Message ID | 20200928170547.13857-1-phil@nwl.cc |
|---|---|
| State | Accepted |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | [iptables] nft: Fix for broken address mask match detection | expand |
On Mon, Sep 28, 2020 at 07:05:47PM +0200, Phil Sutter wrote: > Trying to decide whether a bitwise expression is needed to match parts > of a source or destination address only, add_addr() checks if all bytes > in 'mask' are 0xff or not. The check is apparently broken though as each > byte in 'mask' is cast to a signed char before comparing against 0xff, > therefore the bitwise is always added: > > | # ./bad/iptables-nft -A foo -s 10.0.0.1 -j ACCEPT > | # ./good/iptables-nft -A foo -s 10.0.0.2 -j ACCEPT > | # nft --debug=netlink list chain ip filter foo > | ip filter foo 5 > | [ payload load 4b @ network header + 12 => reg 1 ] > | [ bitwise reg 1 = (reg=1 & 0xffffffff ) ^ 0x00000000 ] > | [ cmp eq reg 1 0x0100000a ] > | [ counter pkts 0 bytes 0 ] > | [ immediate reg 0 accept ] > | > | ip filter foo 6 5 > | [ payload load 4b @ network header + 12 => reg 1 ] > | [ cmp eq reg 1 0x0200000a ] > | [ counter pkts 0 bytes 0 ] > | [ immediate reg 0 accept ] > | > | table ip filter { > | chain foo { > | ip saddr 10.0.0.1 counter packets 0 bytes 0 accept > | ip saddr 10.0.0.2 counter packets 0 bytes 0 accept > | } > | } > > Fix the cast, safe an extra op and gain 100% performance in ideal cases. LGTM.
diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c index c5a8f3fcc051d..7741d23befc5a 100644 --- a/iptables/nft-shared.c +++ b/iptables/nft-shared.c @@ -165,7 +165,7 @@ void add_outiface(struct nftnl_rule *r, char *iface, uint32_t op) void add_addr(struct nftnl_rule *r, int offset, void *data, void *mask, size_t len, uint32_t op) { - const char *m = mask; + const unsigned char *m = mask; int i; add_payload(r, offset, len, NFT_PAYLOAD_NETWORK_HEADER);
Trying to decide whether a bitwise expression is needed to match parts of a source or destination address only, add_addr() checks if all bytes in 'mask' are 0xff or not. The check is apparently broken though as each byte in 'mask' is cast to a signed char before comparing against 0xff, therefore the bitwise is always added: | # ./bad/iptables-nft -A foo -s 10.0.0.1 -j ACCEPT | # ./good/iptables-nft -A foo -s 10.0.0.2 -j ACCEPT | # nft --debug=netlink list chain ip filter foo | ip filter foo 5 | [ payload load 4b @ network header + 12 => reg 1 ] | [ bitwise reg 1 = (reg=1 & 0xffffffff ) ^ 0x00000000 ] | [ cmp eq reg 1 0x0100000a ] | [ counter pkts 0 bytes 0 ] | [ immediate reg 0 accept ] | | ip filter foo 6 5 | [ payload load 4b @ network header + 12 => reg 1 ] | [ cmp eq reg 1 0x0200000a ] | [ counter pkts 0 bytes 0 ] | [ immediate reg 0 accept ] | | table ip filter { | chain foo { | ip saddr 10.0.0.1 counter packets 0 bytes 0 accept | ip saddr 10.0.0.2 counter packets 0 bytes 0 accept | } | } Fix the cast, safe an extra op and gain 100% performance in ideal cases. Fixes: 56859380eb328 ("xtables-compat: avoid unneeded bitwise ops") Signed-off-by: Phil Sutter <phil@nwl.cc> --- iptables/nft-shared.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)