[nf PATCH] netfilter: nft_tproxy: Fix port selector on Big Endian

[nf PATCH] netfilter: nft_tproxy: Fix port selector on Big Endian

[Phil Sutter]

On Big Endian architectures, u16 port value was extracted from the wrong
parts of u32 sreg_port, just like commit 10596608c4d62 ("netfilter:
nf_tables: fix mismatch in big-endian system") describes.

Fixes: 4ed8eb6570a49 ("netfilter: nf_tables: Add native tproxy support")
Cc: Máté Eckl <ecklm94@gmail.com>
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 net/netfilter/nft_tproxy.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nft_tproxy.c b/net/netfilter/nft_tproxy.c
index f92a82c738808..95980154ef02c 100644
--- a/net/netfilter/nft_tproxy.c
+++ b/net/netfilter/nft_tproxy.c
@@ -50,7 +50,7 @@ static void nft_tproxy_eval_v4(const struct nft_expr *expr,
 	taddr = nf_tproxy_laddr4(skb, taddr, iph->daddr);
 
 	if (priv->sreg_port)
-		tport = regs->data[priv->sreg_port];
+		tport = nft_reg_load16(&regs->data[priv->sreg_port]);
 	if (!tport)
 		tport = hp->dest;
 
@@ -117,7 +117,7 @@ static void nft_tproxy_eval_v6(const struct nft_expr *expr,
 	taddr = *nf_tproxy_laddr6(skb, &taddr, &iph->daddr);
 
 	if (priv->sreg_port)
-		tport = regs->data[priv->sreg_port];
+		tport = nft_reg_load16(&regs->data[priv->sreg_port]);
 	if (!tport)
 		tport = hp->dest;
 
-- 
2.24.1

Re: [nf PATCH] netfilter: nft_tproxy: Fix port selector on Big Endian

[Florian Westphal]

Phil Sutter <phil@nwl.cc> wrote:
> On Big Endian architectures, u16 port value was extracted from the wrong
> parts of u32 sreg_port, just like commit 10596608c4d62 ("netfilter:
> nf_tables: fix mismatch in big-endian system") describes.

I was about to debug this today, thanks for debugging/fixing this.

Acked-by: Florian Westphal <fw@strlen.de>

Re: [nf PATCH] netfilter: nft_tproxy: Fix port selector on Big Endian

[Phil Sutter]

Hi,

On Wed, Dec 18, 2019 at 01:03:15AM +0100, Florian Westphal wrote:
> Phil Sutter <phil@nwl.cc> wrote:
> > On Big Endian architectures, u16 port value was extracted from the wrong
> > parts of u32 sreg_port, just like commit 10596608c4d62 ("netfilter:
> > nf_tables: fix mismatch in big-endian system") describes.
> 
> I was about to debug this today, thanks for debugging/fixing this.

With that BE machine at hand, I quickly gave nftables testsuite a try -
results are a bit concerning: The mere fact that netlink debug output
for these immediates differs between BE and LE indicates we don't
seriously test on BE.

Cheers, Phil

Re: [nf PATCH] netfilter: nft_tproxy: Fix port selector on Big Endian

[Florian Westphal]

Phil Sutter <phil@nwl.cc> wrote:
> On Wed, Dec 18, 2019 at 01:03:15AM +0100, Florian Westphal wrote:
> > Phil Sutter <phil@nwl.cc> wrote:
> > > On Big Endian architectures, u16 port value was extracted from the wrong
> > > parts of u32 sreg_port, just like commit 10596608c4d62 ("netfilter:
> > > nf_tables: fix mismatch in big-endian system") describes.
> > 
> > I was about to debug this today, thanks for debugging/fixing this.
> 
> With that BE machine at hand, I quickly gave nftables testsuite a try -
> results are a bit concerning: The mere fact that netlink debug output
> for these immediates differs between BE and LE indicates we don't
> seriously test on BE.

Yes, I fear we will need to add extra .be test files with
big-endian output.

Alternative is to unify debug output in libnftnl to always print
in host byte order, but thats not going to be easy because we don't
know if the immediate value is in network or host byte order.

Re: [nf PATCH] netfilter: nft_tproxy: Fix port selector on Big Endian

[Pablo Neira Ayuso]

On Wed, Dec 18, 2019 at 01:36:25AM +0100, Florian Westphal wrote:
> Phil Sutter <phil@nwl.cc> wrote:
> > On Wed, Dec 18, 2019 at 01:03:15AM +0100, Florian Westphal wrote:
> > > Phil Sutter <phil@nwl.cc> wrote:
> > > > On Big Endian architectures, u16 port value was extracted from the wrong
> > > > parts of u32 sreg_port, just like commit 10596608c4d62 ("netfilter:
> > > > nf_tables: fix mismatch in big-endian system") describes.
> > > 
> > > I was about to debug this today, thanks for debugging/fixing this.
> > 
> > With that BE machine at hand, I quickly gave nftables testsuite a try -
> > results are a bit concerning: The mere fact that netlink debug output
> > for these immediates differs between BE and LE indicates we don't
> > seriously test on BE.
> 
> Yes, I fear we will need to add extra .be test files with
> big-endian output.
> 
> Alternative is to unify debug output in libnftnl to always print
> in host byte order, but thats not going to be easy because we don't
> know if the immediate value is in network or host byte order.

The byteorder information is available in libnftables, so we can
probably move the print function there.

Re: [nf PATCH] netfilter: nft_tproxy: Fix port selector on Big Endian

[Máté Eckl]

On Wed, Dec 18, 2019 at 12:59:29AM +0100, Phil Sutter wrote:
> On Big Endian architectures, u16 port value was extracted from the wrong
> parts of u32 sreg_port, just like commit 10596608c4d62 ("netfilter:
> nf_tables: fix mismatch in big-endian system") describes.
> 
> Fixes: 4ed8eb6570a49 ("netfilter: nf_tables: Add native tproxy support")
> Cc: Máté Eckl <ecklm94@gmail.com>
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---

Acked-by: Máté Eckl <ecklm94@gmail.com>

Thanks for the fix! This was out of my sight.

Re: [nf PATCH] netfilter: nft_tproxy: Fix port selector on Big Endian

[Pablo Neira Ayuso]

On Wed, Dec 18, 2019 at 12:59:29AM +0100, Phil Sutter wrote:
> On Big Endian architectures, u16 port value was extracted from the wrong
> parts of u32 sreg_port, just like commit 10596608c4d62 ("netfilter:
> nf_tables: fix mismatch in big-endian system") describes.

Applied, thanks.