Question about inet_rtm_getroute_build_skb()

Question about inet_rtm_getroute_build_skb()

[Eric Dumazet]

Hi Roopa

I noticed inet_rtm_getroute_build_skb() has this endian issue 
when building an UDP header.

Would the following fix break user space ?

Thanks.

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index a6f20ee3533554b210d27c4ab6637ca7a05b148b..50133b935f868c2ae9474eea027a0ad864a43936 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -3170,7 +3170,7 @@ static struct sk_buff *inet_rtm_getroute_build_skb(__be32 src, __be32 dst,
                udph = skb_put_zero(skb, sizeof(struct udphdr));
                udph->source = sport;
                udph->dest = dport;
-               udph->len = sizeof(struct udphdr);
+               udph->len = htons(sizeof(struct udphdr));
                udph->check = 0;
                break;
        }

Re: Question about inet_rtm_getroute_build_skb()

[David Ahern]

On 8/26/21 6:16 PM, Eric Dumazet wrote:
> Hi Roopa
> 
> I noticed inet_rtm_getroute_build_skb() has this endian issue 
> when building an UDP header.
> 
> Would the following fix break user space ?

I do not see how. As I recall this is only for going through
ip_route_input_rcu and ip_route_output_key_hash_rcu and a call to
fib4_rules_early_flow_dissect.

> 
> Thanks.
> 
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index a6f20ee3533554b210d27c4ab6637ca7a05b148b..50133b935f868c2ae9474eea027a0ad864a43936 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -3170,7 +3170,7 @@ static struct sk_buff *inet_rtm_getroute_build_skb(__be32 src, __be32 dst,
>                 udph = skb_put_zero(skb, sizeof(struct udphdr));
>                 udph->source = sport;
>                 udph->dest = dport;
> -               udph->len = sizeof(struct udphdr);
> +               udph->len = htons(sizeof(struct udphdr));
>                 udph->check = 0;
>                 break;
>         }
> 

Re: Question about inet_rtm_getroute_build_skb()

[Eric Dumazet]

On 8/26/21 9:37 PM, David Ahern wrote:
> On 8/26/21 6:16 PM, Eric Dumazet wrote:
>> Hi Roopa
>>
>> I noticed inet_rtm_getroute_build_skb() has this endian issue 
>> when building an UDP header.
>>
>> Would the following fix break user space ?
> 
> I do not see how. As I recall this is only for going through
> ip_route_input_rcu and ip_route_output_key_hash_rcu and a call to
> fib4_rules_early_flow_dissect.
> 

Ah, nice !

Could we add a test for this feature ?
I could not really figure out reading the commit changelog how this stuff was used.

Re: Question about inet_rtm_getroute_build_skb()

[David Ahern]

On 8/27/21 8:09 AM, Eric Dumazet wrote:
> 
> 
> On 8/26/21 9:37 PM, David Ahern wrote:
>> On 8/26/21 6:16 PM, Eric Dumazet wrote:
>>> Hi Roopa
>>>
>>> I noticed inet_rtm_getroute_build_skb() has this endian issue 
>>> when building an UDP header.
>>>
>>> Would the following fix break user space ?
>>
>> I do not see how. As I recall this is only for going through
>> ip_route_input_rcu and ip_route_output_key_hash_rcu and a call to
>> fib4_rules_early_flow_dissect.
>>
> 
> Ah, nice !
> 
> Could we add a test for this feature ?
> I could not really figure out reading the commit changelog how this stuff was used.
> 

tools/testing/selftests/net/fib_rule_tests.sh

I do not see the udp variant, so yes, those should be added.