[PATCH iproute2-next] tc: nat: Fix mask calculation

[PATCH iproute2-next] tc: nat: Fix mask calculation

[Torben Nielsen]

In parse_nat_args the network mask is calculated as

        sel->mask = htonl(~0u << (32 - addr.bitlen));

According to  ISO/IEC 9899:TC3 6.5.7 Bitwise shift operators:
The integer promotions are performed on each of the operands.
The type of the result is that of the promoted left operand.
If the value of the right operand is negative or is greater
than or equal to the width of the promoted left operand,
the behavior is undefined

Specifically this means that the mask is undefined for
addr.bitlen = 0
On x86_64 the result is 0xffffffff, on armv7l it is 0.

Promoting the left operand of the shift operator solves this issue.

Signed-off-by: Torben Nielsen <torben.nielsen@prevas.dk>
---
 tc/m_nat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tc/m_nat.c b/tc/m_nat.c
index 69d54c6f..da947aea 100644
--- a/tc/m_nat.c
+++ b/tc/m_nat.c
@@ -55,7 +55,7 @@ parse_nat_args(int *argc_p, char ***argv_p, struct tc_nat *sel)
 		goto bad_val;
 
 	sel->old_addr = addr.data[0];
-	sel->mask = htonl(~0u << (32 - addr.bitlen));
+	sel->mask = htonl(~(uint64_t)0 << (32 - addr.bitlen));
 
 	NEXT_ARG();
 
-- 
2.43.0

[PATCH iproute2-next] tc: nat: ffs should operation on host byte ordered data

[Torben Nielsen]

In print_nat the mask length is calculated as

	len = ffs(sel->mask);
	len = len ? 33 - len : 0;

The mask is stored in network byte order, it should be converted
to host byte order before calculating first bit set.
---
 tc/m_nat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tc/m_nat.c b/tc/m_nat.c
index da947aea..0ec3fd11 100644
--- a/tc/m_nat.c
+++ b/tc/m_nat.c
@@ -156,7 +156,7 @@ print_nat(const struct action_util *au, FILE * f, struct rtattr *arg)
 	}
 	sel = RTA_DATA(tb[TCA_NAT_PARMS]);
 
-	len = ffs(sel->mask);
+	len = ffs(ntohl(sel->mask));
 	len = len ? 33 - len : 0;
 
 	print_string(PRINT_ANY, "direction", "%s",
-- 
2.43.0

Re: [PATCH iproute2-next] tc: nat: ffs should operation on host byte ordered data

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to iproute2/iproute2.git (main)
by Stephen Hemminger <stephen@networkplumber.org>:

On Thu,  6 Mar 2025 12:25:20 +0100 you wrote:
> In print_nat the mask length is calculated as
> 
> 	len = ffs(sel->mask);
> 	len = len ? 33 - len : 0;
> 
> The mask is stored in network byte order, it should be converted
> to host byte order before calculating first bit set.
> 
> [...]

Here is the summary with links:
  - [iproute2-next] tc: nat: ffs should operation on host byte ordered data
    https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=afbfd2f2b0a6

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH iproute2-next] tc: nat: Fix mask calculation

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to iproute2/iproute2.git (main)
by Stephen Hemminger <stephen@networkplumber.org>:

On Thu,  6 Mar 2025 12:25:19 +0100 you wrote:
> In parse_nat_args the network mask is calculated as
> 
>         sel->mask = htonl(~0u << (32 - addr.bitlen));
> 
> According to  ISO/IEC 9899:TC3 6.5.7 Bitwise shift operators:
> The integer promotions are performed on each of the operands.
> The type of the result is that of the promoted left operand.
> If the value of the right operand is negative or is greater
> than or equal to the width of the promoted left operand,
> the behavior is undefined
> 
> [...]

Here is the summary with links:
  - [iproute2-next] tc: nat: Fix mask calculation
    https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=667817b4c349

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html