Re: [PATCH v9 0/4] Reconstruct incoming ICMP headers for failed UDP connect and forward back

[Stefano Brivio <sbrivio@redhat.com>]

On Mon,  3 Mar 2025 20:29:11 -0500
Jon Maloy <jmaloy@redhat.com> wrote:

> v2: - Added patch breaking out udp header creation from function
>       tap_udp4_send().
>     - Updated the ICMP creation by using the new function.
>     - Added logics to find correct flow, depending on origin.
>     - All done after feedback from David Gibson.
> v3: - More changes after feedback from David Gibson.
> v4: - Even more changes after feedback from D. Gibson
> v5: - Added corresponding patches for IPv6
> v6: - Fixed some small nits after comments from D. Gibson.
> v7: - Added handling of all rejected ICMP messages
>     - Returning correct user data amount if IPv6 as per RFC 4884.
> v8: - Added MTU to ICMPv4 ICMP_FRAG_NEEDED messages.
>     - Added ASSERT() validation to message creation functions.
> v9: - Using real source address of ICMP to complement destination
>       address for originial UDP message when needed.
> 
> Jon Maloy (4):
>   tap: break out building of udp header from tap_udp4_send function
>   udp: create and send ICMPv4 to local peer when applicable
>   tap: break out building of udp header from tap_udp6_send function
>   udp: create and send ICMPv6 to local peer when applicable

I was about to apply those, then I realised that Coverity Scan isn't
happy about a few things, listed below. I didn't check if those are
false positives (I can have a look later or within a couple of days
unless you get to it first).

1.
---
/home/sbrivio/passt/udp.c:448:2:
  Type: Out-of-bounds access (ARRAY_VS_SINGLETON)

/home/sbrivio/passt/udp.c:440:2:
  1. path: Condition "!(dlen <= 8)", taking false branch.
/home/sbrivio/passt/udp.c:444:2:
  2. path: Condition "ee->ee_type == 3", taking true branch.
/home/sbrivio/passt/udp.c:444:2:
  3. path: Condition "ee->ee_code == 4", taking true branch.
/home/sbrivio/passt/udp.c:448:2:
  4. address_of: Taking address with "&msg.ip4h" yields a singleton pointer.
/home/sbrivio/passt/udp.c:448:2:
  5. callee_ptr_arith: Passing "&msg.ip4h" to function "tap_push_ip4h" which uses it as an array. This might corrupt or misinterpret adjacent memory locations.
/home/sbrivio/passt/tap.c:162:2:
  5.1. ptr_arith: Performing pointer arithmetic on "ip4h" in expression "ip4h + 1".
---

2.
---
/home/sbrivio/passt/udp.c:493:2:
  Type: Out-of-bounds access (ARRAY_VS_SINGLETON)

/home/sbrivio/passt/udp.c:485:2:
  1. path: Condition "!(dlen <= 1232UL /* 1280 - sizeof (struct udphdr) - sizeof (struct ipv6hdr) */)", taking false branch.
/home/sbrivio/passt/udp.c:489:2:
  2. path: Condition "ee->ee_type == 2", taking true branch.
/home/sbrivio/passt/udp.c:493:2:
  3. address_of: Taking address with "&msg.ip6h" yields a singleton pointer.
/home/sbrivio/passt/udp.c:493:2:
  4. callee_ptr_arith: Passing "&msg.ip6h" to function "tap_push_ip6h" which uses it as an array. This might corrupt or misinterpret adjacent memory locations.
/home/sbrivio/passt/tap.c:265:2:
  4.1. ptr_arith: Performing pointer arithmetic on "ip6h" in expression "ip6h + 1".
---

3.
---
/home/sbrivio/passt/udp.c:449:2:
  Type: Out-of-bounds access (ARRAY_VS_SINGLETON)

/home/sbrivio/passt/udp.c:440:2:
  1. path: Condition "!(dlen <= 8)", taking false branch.
/home/sbrivio/passt/udp.c:444:2:
  2. path: Condition "ee->ee_type == 3", taking true branch.
/home/sbrivio/passt/udp.c:444:2:
  3. path: Condition "ee->ee_code == 4", taking true branch.
/home/sbrivio/passt/udp.c:449:2:
  4. address_of: Taking address with "&msg.uh" yields a singleton pointer.
/home/sbrivio/passt/udp.c:449:2:
  5. callee_ptr_arith: Passing "&msg.uh" to function "tap_push_uh4" which uses it as an array. This might corrupt or misinterpret adjacent memory locations.
/home/sbrivio/passt/tap.c:190:2:
  5.1. ptr_arith: Performing pointer arithmetic on "uh" in expression "uh + 1".
---

4.
---
/home/sbrivio/passt/udp.c:494:2:
  Type: Out-of-bounds access (ARRAY_VS_SINGLETON)

/home/sbrivio/passt/udp.c:485:2:
  1. path: Condition "!(dlen <= 1232UL /* 1280 - sizeof (struct udphdr) - sizeof (struct ipv6hdr) */)", taking false branch.
/home/sbrivio/passt/udp.c:489:2:
  2. path: Condition "ee->ee_type == 2", taking true branch.
/home/sbrivio/passt/udp.c:494:2:
  3. address_of: Taking address with "&msg.uh" yields a singleton pointer.
/home/sbrivio/passt/udp.c:494:2:
  4. callee_ptr_arith: Passing "&msg.uh" to function "tap_push_uh6" which uses it as an array. This might corrupt or misinterpret adjacent memory locations.
/home/sbrivio/passt/tap.c:295:2:
  4.1. ptr_arith: Performing pointer arithmetic on "uh" in expression "uh + 1".
---

-- 
Stefano